[PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

In the gimple-ssa-sprintf pass I made the incorrect assumption that
a wider integer argument in some range [X, Y] to a narrower directive
(such as int to %hhi) results in the number of bytes corresponding to
the range bounded by the number of bytes resulting from formatting X
and Y converted to the directive's type (how's that for a clear
description?)

Basically, given an int A in [X, Y], and sprintf("%hhi", A) the logic
was to transform the range to [X', Y'] where X' = (signed char)X and
Y' = (signed char)Y, compute a range of bytes [B, C] that X' and Y'
would format to, and use that as the range for A.  That's wrong when
X or Y are outside the range of the directive's type because of
overflow or wrapping.  It's possible to find B in [X, Y] such that
(signed char)B is outside the range of [X', Y'].

Bug 78622 - [7 Regression] -Wformat-length/-fprintf-return-value
incorrect with overflow/wrapping, derived from a comment on bug
78586 has a test case.

The attached patch fixes this problem.  A happy consequence of
the fix is that it also resolves bug 77721 - -Wformat-length not
uses arg range for converted vars (or at least makes the test
case in the bug pass; there are outstanding limitations due
to poor range info in the pass).

While there, I also fixed a couple of minor cosmetic issues with
the phrasing and formatting of diagnostics (unrelated to the main
problem).

Tested on x86-64.

Thanks
Martin

[-- Attachment #2: gcc-78622.diff --]
[-- Type: text/x-patch, Size: 14555 bytes --]

PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping

gcc/ChangeLog:

	PR middle-end/78622
	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
	rather than res.bounded.
	(adjust_range_for_overflow): New function.
	(format_integer): Always set res.bounded to true unless either
	precision or width is specified and unknown.
	Call adjust_range_for_overflow.
	(format_directive): Remove vestigial quoting.
	(add_bytes): Correct the computation of boundrange used to
	decide whether a warning is of a "maybe" or "defnitely" kind.

gcc/testsuite/ChangeLog:

	PR middle-end/78622
	* gcc.c-torture/execute/pr78622.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
	behavior inadvertently introduced in a previous commit.  Tighten
	up final checking.
	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
	add a final optimization check.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 99a635a..eceed3e 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -637,7 +637,7 @@ min_bytes_remaining (unsigned HOST_WIDE_INT navail, const format_result &res)
   if (HOST_WIDE_INT_MAX <= navail)
     return navail;
 
-  if (1 < warn_format_length || res.bounded)
+  if (1 < warn_format_length || res.knownrange)
     {
       /* At level 2, or when all directives output an exact number
 	 of bytes or when their arguments were bounded by known
@@ -795,6 +795,43 @@ get_width_and_precision (const conversion_spec &spec,
   *pprec = prec;
 }
 
+/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
+   argument, due to the conversion from either *ARGMIN or *ARGMAX to
+   the type of the directive's formal argument it's possible for both
+   to result in the same number of bytes or a range of bytes that's
+   less than the number of bytes that would result from formatting
+   some other value in the range [*ARGMIN, *ARGMAX].  This can be
+   determined by checking for the actual argument being in the range
+   of the type of the directive.  If it isn't it must be assumed to
+   take on the full range of the directive's type.
+   Return true when the range has been adjusted, false otherwise.  */
+
+static bool
+adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
+{
+  tree dirmin = TYPE_MIN_VALUE (dirtype);
+  tree dirmax = TYPE_MAX_VALUE (dirtype);
+
+  if (tree_int_cst_lt (*argmin, dirmin)
+      || tree_int_cst_lt (dirmax, *argmin)
+      || tree_int_cst_lt (*argmax, dirmin)
+      || tree_int_cst_lt (dirmax, *argmax))
+    {
+      if (TYPE_UNSIGNED (dirtype))
+	{
+	  *argmin = dirmin;
+	  *argmax = dirmax;
+	}
+      else
+	{
+	  *argmin = integer_zero_node;
+	  *argmax = dirmin;
+	}
+      return true;
+    }
+  return false;
+}
+
 /* Return a range representing the minimum and maximum number of bytes
    that the conversion specification SPEC will write on output for the
    integer argument ARG when non-null.  ARG may be null (for vararg
@@ -989,6 +1026,10 @@ format_integer (const conversion_spec &spec, tree arg)
 
   fmtresult res;
 
+  /* The result is bounded unless width or precision has been specified
+     whose value is unknown.  */
+  res.bounded = width != HOST_WIDE_INT_MIN && prec != HOST_WIDE_INT_MIN;
+
   /* Using either the range the non-constant argument is in, or its
      type (either "formal" or actual), create a range of values that
      constrain the length of output given the warning level.  */
@@ -1010,6 +1051,12 @@ format_integer (const conversion_spec &spec, tree arg)
 	  res.argmax = build_int_cst (argtype, wi::fits_uhwi_p (max)
 				      ? max.to_uhwi () : max.to_shwi ());
 
+	  /* Set KNOWNRANGE if the argument is in a known subrange
+	     of the directive's type (KNOWNRANGE may be reset below).  */
+	  res.knownrange
+	    = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), res.argmin)
+	       && !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), res.argmax));
+
 	  /* For a range with a negative lower bound and a non-negative
 	     upper bound, use one to determine the minimum number of bytes
 	     on output and whichever of the two bounds that results in
@@ -1020,6 +1067,10 @@ format_integer (const conversion_spec &spec, tree arg)
 	    {
 	      argmin = res.argmin;
 	      argmax = res.argmax;
+
+	      res.knownrange
+		&= !adjust_range_for_overflow (dirtype, &argmin, &argmax);
+
 	      int minbytes = format_integer (spec, res.argmin).range.min;
 	      int maxbytes = format_integer (spec, res.argmax).range.max;
 	      if (maxbytes < minbytes)
@@ -1032,11 +1083,6 @@ format_integer (const conversion_spec &spec, tree arg)
 	      argmin = res.argmin;
 	      argmax = res.argmax;
 	    }
-
-	  /* The argument is bounded by the known range of values
-	     determined by Value Range Propagation.  */
-	  res.bounded = true;
-	  res.knownrange = true;
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
@@ -1101,6 +1147,10 @@ format_integer (const conversion_spec &spec, tree arg)
       res.argmax = argmax;
     }
 
+  /* Clear KNOWNRANGE if the range has been adjusted to the maximum
+     of the directive.  */
+  res.knownrange &= !adjust_range_for_overflow (dirtype, &argmin, &argmax);
+
   /* Recursively compute the minimum and maximum from the known range,
      taking care to swap them if the lower bound results in longer
      output than the upper bound (e.g., in the range [-1, 0].  */
@@ -1879,7 +1929,7 @@ format_directive (const pass_sprintf_length::call_info &info,
 			fmtres.argmin, fmtres.argmax);
 	      else
 		inform (info.fmtloc,
-			"using the range [%qE, %qE] for directive argument",
+			"using the range [%E, %E] for directive argument",
 			fmtres.argmin, fmtres.argmax);
 	    }
 	}
@@ -2057,7 +2107,7 @@ add_bytes (const pass_sprintf_length::call_info &info,
 	 indicate that the overflow/truncation may (but need not) happen.  */
       bool boundrange
 	= (res->number_chars_min < res->number_chars_max
-	   && res->number_chars_min < info.objsize);
+	   && res->number_chars_min + nbytes <= info.objsize);
 
       if (!end && ((nbytes - navail) == 1 || boundrange))
 	{
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78622.c b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
new file mode 100644
index 0000000..44c9b0b
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
@@ -0,0 +1,39 @@
+/* PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value
+   incorrect with overflow/wrapping */
+/* { dg-additional-options "-Wformat-length=2" } */
+
+__attribute__((noinline, noclone)) int
+foo (int x)
+{
+  if (x < 4096 + 8 || x >= 4096 + 256 + 8)
+    return -1;
+  char buf[5];
+  return __builtin_sprintf (buf, "%hhd", x + 1);
+}
+
+int
+bar (unsigned int x)
+{
+  if (x < 64 || x > 2U * __INT_MAX__ - 10)
+    return -1;
+  char buf[1];
+  return __builtin_sprintf (buf, "%d", x + 1);   /* { dg-warn "directive writing between 1 and 11 bytes into a region of size 1" "int32plus" { target { int32plus } } */
+}
+
+int
+main ()
+{
+  if (__SCHAR_MAX__ != 127 || __CHAR_BIT__ != 8 || __SIZEOF_INT__ != 4)
+    return 0;
+  if (foo (4095 + 9) != 1
+      || foo (4095 + 32) != 2
+      || foo (4095 + 127) != 3
+      || foo (4095 + 128) != 4
+      || foo (4095 + 240) != 3
+      || foo (4095 + 248) != 2
+      || foo (4095 + 255) != 2
+      || foo (4095 + 256) != 1)
+    __builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
index 4dddccdf..077c33b 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
@@ -75,6 +75,8 @@ EQL (0, 0, "%-s", "");
 EQL (1, 1, "%c",  'x');
 EQL (1, 1, "%-s", "x");
 
+EQL (1, 2, "%c",  'x');
+
 EQL (4, 4, "%4c", 'x');
 
 /* Verify that exceeding the environmental limit of 4095 bytes for
@@ -179,7 +181,7 @@ RNG (4,  4, 32, "%zu", sz)
 
 /* Exercise bug 78586.  */
 RNG (4,  4, 32, "%lu", (unsigned long)i)
-RNG (4,  4, 32, "%lu", (unsigned)u)
+RNG (4,  4, 32, "%lu", (unsigned long)u)
 RNG (4,  4, 32, "%lu", (unsigned long)li)
 RNG (4,  4, 32, "%lu", (unsigned long)lu)
 RNG (4,  4, 32, "%lu", (unsigned long)sz)
@@ -266,14 +268,17 @@ RNG (0,  6,   8, "%s%ls", "1", L"2");
    <bb 2>:
    result_3 = __builtin_sprintf (&MEM[(void *)&buf8k + 8192B], "%c", 32);
    if (result_3 != 0)
-     goto <bb 3>;
+     goto <bb 3>; [50.0%]
    else
-     goto <bb 4>;
+     goto <bb 4>; [50.0%]
 
-   <bb 3>:
+   <bb 3>[50.0%]:
    __builtin_abort ();
 
 */
 
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 124 "optimized" { target { ilp32 || lp64 } } } } */
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } } */
+/*  Only conditional calls to abort should be made (with any probability:
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 124 "optimized" { target { ilp32 || lp64 } } } }
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } }
+    No unconditional calls to abort should be made:
+    { dg-final { scan-tree-dump-not ";\n *__builtin_abort" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
index 4c41234..d1013e5 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
@@ -50,6 +50,14 @@ void test_arg_int (int width, int prec, int i, int n)
 
   T ("%i", R (1, 10));
 
+  /* Each of the bounds of the ranges below results in just one byte
+     on output because they convert to the value 9 in type char, yet
+     other values in those ranges can result in up to four bytes.
+     For example, 4240 converts to -112.  Verify that the output
+     isn't folded into a constant.  */
+  T ("%hhi", R (4104, 4360) + 1);
+  T ("%hhu", R (4104, 4360) + 1);
+
   T ("%'i", 1234567);
 
   for (i = -n; i != n; ++i)
@@ -106,4 +114,4 @@ void test_invalid_directive (void)
 
 /* Use 'grep "^ *T (" builtin-sprintf-6.c  | wc -l' to determine
    the count for the directive below.
-   { dg-final { scan-tree-dump-times "snprintf" 42 "optimized"} } */
+   { dg-final { scan-tree-dump-times "snprintf" 44 "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
index fae584e..f49422a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -1039,8 +1039,8 @@ void test_sprintf_chk_s_nonconst (int w, int p, const char *s)
      is not.  */
   T ( 1, "%*s",  w, "");
   T ( 1, "%*s",  w, "1");       /* { dg-warning "nul past the end" } */
-  T ( 1, "%.*s", w, "");
-  T ( 1, "%.*s", w, "1");       /* { dg-warning "may write a terminating nul" } */
+  T ( 1, "%.*s", p, "");
+  T ( 1, "%.*s", p, "1");       /* { dg-warning "may write a terminating nul" } */
   T ( 1, "%.*s", w, "123");     /* { dg-warning "writing between 0 and 3 bytes into a region of size 1" } */
 
   T ( 1, "%*s", w, "123");      /* { dg-warning "writing 3 or more bytes into a region of size 1" } */
@@ -1294,6 +1294,12 @@ void test_sprintf_chk_int_nonconst (int w, int p, int a)
   T (3, "%2x",          a);
 
   T (1, "%.*d",      p, a);
+
+  T (4, "%i %i",        a, a);
+  /* The following will definitely be "writing a terminating nul past the end"
+     (i.e., not "may write".)  */
+  T (4, "%i %i ",       a, a);      /* { dg-warning "writing a terminating nul past the end" } */
+  T (4, "%i %i %i",     a, a, a);   /* { dg-warning "into a region" }*/
 }
 
 void test_sprintf_chk_e_nonconst (int w, int p, double d)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
index 0cb02b7..121ed4e 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
@@ -2,15 +2,18 @@
    Test to verify that the correct range information is made available to the
    -Wformat-lenght check to prevent warnings.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wformat -Wformat-length" } */
+/* { dg-options "-O2 -Wformat -Wformat-length -fdump-tree-optimized" } */
 
+void abort (void);
 int snprintf (char*, __SIZE_TYPE__, const char*, ...);
 
 void fuchar (unsigned char j, char *p)
 {
   if (j > 99)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fschar (signed char j, char *p)
@@ -20,14 +23,17 @@ void fschar (signed char j, char *p)
   if (k > 99)
     return;
 
-  snprintf (p, 3, "%3hhu", k);   /* { dg-bogus "" "unsigned char" { xfail *-*-* } } */
+  if (3 != snprintf (p, 4, "%3hhu", k))
+    abort ();
 }
 
 void fushrt (unsigned short j, char *p)
 {
   if (j > 999)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fshrt (short j, char *p)
@@ -37,7 +43,8 @@ void fshrt (short j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3hu", k);
+  if (3 != snprintf (p, 4, "%3hu", k))
+    abort ();
 }
 
 void fuint (unsigned j, char *p)
@@ -54,13 +61,16 @@ void fint (int j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3u", k);   /* { dg-bogus "" "unsigned int" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3u", k);
 }
 
 void fulong (unsigned long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3lu", j);
 }
 
@@ -71,13 +81,16 @@ void flong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3lu", k);   /* { dg-bogus "" "unsigned long" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3lu", k);
 }
 
 void fullong (unsigned long long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3llu", j);
 }
 
@@ -88,5 +101,7 @@ void fllong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3llu", k);   /* { dg-bogus "" "unsigned long long" { xfail lp64 } } */
+  snprintf (p, 4, "%3llu", k);
 }
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jakub Jelinek]

On Wed, Nov 30, 2016 at 08:26:04PM -0700, Martin Sebor wrote:
> @@ -795,6 +795,43 @@ get_width_and_precision (const conversion_spec &spec,
>    *pprec = prec;
>  }
>  
> +/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
> +   argument, due to the conversion from either *ARGMIN or *ARGMAX to
> +   the type of the directive's formal argument it's possible for both
> +   to result in the same number of bytes or a range of bytes that's
> +   less than the number of bytes that would result from formatting
> +   some other value in the range [*ARGMIN, *ARGMAX].  This can be
> +   determined by checking for the actual argument being in the range
> +   of the type of the directive.  If it isn't it must be assumed to
> +   take on the full range of the directive's type.
> +   Return true when the range has been adjusted, false otherwise.  */
> +
> +static bool
> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
> +{
> +  tree dirmin = TYPE_MIN_VALUE (dirtype);
> +  tree dirmax = TYPE_MAX_VALUE (dirtype);
> +
> +  if (tree_int_cst_lt (*argmin, dirmin)
> +      || tree_int_cst_lt (dirmax, *argmin)
> +      || tree_int_cst_lt (*argmax, dirmin)
> +      || tree_int_cst_lt (dirmax, *argmax))
> +    {
> +      if (TYPE_UNSIGNED (dirtype))
> +	{
> +	  *argmin = dirmin;
> +	  *argmax = dirmax;
> +	}
> +      else
> +	{
> +	  *argmin = integer_zero_node;
> +	  *argmax = dirmin;
> +	}
> +      return true;
> +    }

Isn't this too simplistic?  I mean, if you have say dirtype of signed char
and argmin say 4096 + 32 and argmax say 4096 + 64, (signed char) arg
has range 32, 64, while I think your routine will yield -128, 127 (well,
0 as min and -128 as max as that is what you return for signed type).

Can't you subtract argmax - argmin (best just in wide_int, no need to build
trees), and use what you have just for the case where that number doesn't
fit into the narrower precision, otherwise if argmin - (dirtype) argmin
== argmax - (dirtype) argmax, just use (dirtype) argmin and (dirtype) argmax
as the range, and in case that it crosses a boundary figure if you can do
anything than the above?  Guess all cases of signed/unsigned dirtype and/or
argtype need to be considered.

Also, is argmin and argmax in this case the actual range (what should go
into res.arg{min,max}), or the values with shortest/longest representation?
Wouldn't it be better to always compute the range of values that can be
printed and only later on (after all VR_RANGE and VR_VARYING handling)
transform that into the number with shortest/longest representation in that
range?  Perhaps even using different variable names for the latter would
make things clearer (argshortest, arglongest or whatever).

	Jakub

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jakub Jelinek]

On Thu, Dec 01, 2016 at 08:26:47AM +0100, Jakub Jelinek wrote:
> Isn't this too simplistic?  I mean, if you have say dirtype of signed char
> and argmin say 4096 + 32 and argmax say 4096 + 64, (signed char) arg
> has range 32, 64, while I think your routine will yield -128, 127 (well,
> 0 as min and -128 as max as that is what you return for signed type).
> 
> Can't you subtract argmax - argmin (best just in wide_int, no need to build
> trees), and use what you have just for the case where that number doesn't
> fit into the narrower precision, otherwise if argmin - (dirtype) argmin
> == argmax - (dirtype) argmax, just use (dirtype) argmin and (dirtype) argmax
> as the range, and in case that it crosses a boundary figure if you can do
> anything than the above?  Guess all cases of signed/unsigned dirtype and/or
> argtype need to be considered.

Richard noted that you could have a look at CONVERT_EXPR_CODE_P
handling in extract_range_from_unary_expr.  I think it is the
              || (vr0.type == VR_RANGE
                  && integer_zerop (int_const_binop (RSHIFT_EXPR,
                       int_const_binop (MINUS_EXPR, vr0.max, vr0.min),
                         size_int (TYPE_PRECISION (outer_type)))))))
part that is important here for the narrowing conversion.

	Jakub

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On 12/01/2016 02:15 AM, Jakub Jelinek wrote:
> On Thu, Dec 01, 2016 at 08:26:47AM +0100, Jakub Jelinek wrote:
>> Isn't this too simplistic?  I mean, if you have say dirtype of signed char
>> and argmin say 4096 + 32 and argmax say 4096 + 64, (signed char) arg
>> has range 32, 64, while I think your routine will yield -128, 127 (well,
>> 0 as min and -128 as max as that is what you return for signed type).
>>
>> Can't you subtract argmax - argmin (best just in wide_int, no need to build
>> trees), and use what you have just for the case where that number doesn't
>> fit into the narrower precision, otherwise if argmin - (dirtype) argmin
>> == argmax - (dirtype) argmax, just use (dirtype) argmin and (dirtype) argmax
>> as the range, and in case that it crosses a boundary figure if you can do
>> anything than the above?  Guess all cases of signed/unsigned dirtype and/or
>> argtype need to be considered.
>
> Richard noted that you could have a look at CONVERT_EXPR_CODE_P
> handling in extract_range_from_unary_expr.  I think it is the
>               || (vr0.type == VR_RANGE
>                   && integer_zerop (int_const_binop (RSHIFT_EXPR,
>                        int_const_binop (MINUS_EXPR, vr0.max, vr0.min),
>                          size_int (TYPE_PRECISION (outer_type)))))))
> part that is important here for the narrowing conversion.

Thanks, that was a helpful suggestion!  Attached is an update that
follows the vrp approach.  I assume the infinity stuff is specific
to the VRP pass and not something I need to worry about here.

Martin

PS To your earlier question, argmin and argmax have the same meaning
in the added helper function as in its caller.

[-- Attachment #2: gcc-78622.diff --]
[-- Type: text/x-patch, Size: 26767 bytes --]

PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping

gcc/ChangeLog:

	PR middle-end/78622
	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
	rather than res.bounded.
	(adjust_range_for_overflow): New function.
	(format_integer): Always set res.bounded to true unless either
	precision or width is specified and unknown.
	Call adjust_range_for_overflow.
	(format_directive): Remove vestigial quoting.
	(add_bytes): Correct the computation of boundrange used to
	decide whether a warning is of a "maybe" or "defnitely" kind.

gcc/testsuite/ChangeLog:

	PR middle-end/78622
	* gcc.c-torture/execute/pr78622.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
	behavior inadvertently introduced in a previous commit.  Tighten
	up final checking.
	* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
	Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
	add a final optimization check.
	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 99a635a..95a8710 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -637,7 +637,7 @@ min_bytes_remaining (unsigned HOST_WIDE_INT navail, const format_result &res)
   if (HOST_WIDE_INT_MAX <= navail)
     return navail;
 
-  if (1 < warn_format_length || res.bounded)
+  if (1 < warn_format_length || res.knownrange)
     {
       /* At level 2, or when all directives output an exact number
 	 of bytes or when their arguments were bounded by known
@@ -795,6 +795,59 @@ get_width_and_precision (const conversion_spec &spec,
   *pprec = prec;
 }
 
+/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
+   argument, due to the conversion from either *ARGMIN or *ARGMAX to
+   the type of the directive's formal argument it's possible for both
+   to result in the same number of bytes or a range of bytes that's
+   less than the number of bytes that would result from formatting
+   some other value in the range [*ARGMIN, *ARGMAX].  This can be
+   determined by checking for the actual argument being in the range
+   of the type of the directive.  If it isn't it must be assumed to
+   take on the full range of the directive's type.
+   Return true when the range has been adjusted to the range of
+   DIRTYPE, false otherwise.  */
+
+static bool
+adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
+{
+  unsigned argprec = TYPE_PRECISION (TREE_TYPE (*argmin));
+  unsigned dirprec = TYPE_PRECISION (dirtype);
+
+  /* The logic here was inspired/lifted from  the CONVERT_EXPR_CODE_P
+     branch in the extract_range_from_unary_expr function in tree-vrp.c.
+  */
+
+  if (TREE_CODE (*argmin) == INTEGER_CST
+      && TREE_CODE (*argmax) == INTEGER_CST
+      && (dirprec >= argprec
+	  || integer_zerop (int_const_binop (RSHIFT_EXPR,
+					     int_const_binop (MINUS_EXPR,
+							      *argmax,
+							      *argmin),
+					     size_int (dirprec)))))
+  {
+    *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0, false);
+    *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0, false);
+    return false;
+  }
+
+  tree dirmin = TYPE_MIN_VALUE (dirtype);
+  tree dirmax = TYPE_MAX_VALUE (dirtype);
+
+  if (TYPE_UNSIGNED (dirtype))
+    {
+      *argmin = dirmin;
+      *argmax = dirmax;
+    }
+  else
+    {
+      *argmin = integer_zero_node;
+      *argmax = dirmin;
+    }
+
+  return true;
+}
+
 /* Return a range representing the minimum and maximum number of bytes
    that the conversion specification SPEC will write on output for the
    integer argument ARG when non-null.  ARG may be null (for vararg
@@ -989,6 +1042,10 @@ format_integer (const conversion_spec &spec, tree arg)
 
   fmtresult res;
 
+  /* The result is bounded unless width or precision has been specified
+     whose value is unknown.  */
+  res.bounded = width != HOST_WIDE_INT_MIN && prec != HOST_WIDE_INT_MIN;
+
   /* Using either the range the non-constant argument is in, or its
      type (either "formal" or actual), create a range of values that
      constrain the length of output given the warning level.  */
@@ -1010,6 +1067,12 @@ format_integer (const conversion_spec &spec, tree arg)
 	  res.argmax = build_int_cst (argtype, wi::fits_uhwi_p (max)
 				      ? max.to_uhwi () : max.to_shwi ());
 
+	  /* Set KNOWNRANGE if the argument is in a known subrange
+	     of the directive's type (KNOWNRANGE may be reset below).  */
+	  res.knownrange
+	    = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), res.argmin)
+	       && !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), res.argmax));
+
 	  /* For a range with a negative lower bound and a non-negative
 	     upper bound, use one to determine the minimum number of bytes
 	     on output and whichever of the two bounds that results in
@@ -1020,6 +1083,10 @@ format_integer (const conversion_spec &spec, tree arg)
 	    {
 	      argmin = res.argmin;
 	      argmax = res.argmax;
+
+	      res.knownrange
+		&= !adjust_range_for_overflow (dirtype, &argmin, &argmax);
+
 	      int minbytes = format_integer (spec, res.argmin).range.min;
 	      int maxbytes = format_integer (spec, res.argmax).range.max;
 	      if (maxbytes < minbytes)
@@ -1032,11 +1099,6 @@ format_integer (const conversion_spec &spec, tree arg)
 	      argmin = res.argmin;
 	      argmax = res.argmax;
 	    }
-
-	  /* The argument is bounded by the known range of values
-	     determined by Value Range Propagation.  */
-	  res.bounded = true;
-	  res.knownrange = true;
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
@@ -1101,6 +1163,10 @@ format_integer (const conversion_spec &spec, tree arg)
       res.argmax = argmax;
     }
 
+  /* Clear KNOWNRANGE if the range has been adjusted to the maximum
+     of the directive.  */
+  res.knownrange &= !adjust_range_for_overflow (dirtype, &argmin, &argmax);
+
   /* Recursively compute the minimum and maximum from the known range,
      taking care to swap them if the lower bound results in longer
      output than the upper bound (e.g., in the range [-1, 0].  */
@@ -1879,7 +1945,7 @@ format_directive (const pass_sprintf_length::call_info &info,
 			fmtres.argmin, fmtres.argmax);
 	      else
 		inform (info.fmtloc,
-			"using the range [%qE, %qE] for directive argument",
+			"using the range [%E, %E] for directive argument",
 			fmtres.argmin, fmtres.argmax);
 	    }
 	}
@@ -2057,7 +2123,7 @@ add_bytes (const pass_sprintf_length::call_info &info,
 	 indicate that the overflow/truncation may (but need not) happen.  */
       bool boundrange
 	= (res->number_chars_min < res->number_chars_max
-	   && res->number_chars_min < info.objsize);
+	   && res->number_chars_min + nbytes <= info.objsize);
 
       if (!end && ((nbytes - navail) == 1 || boundrange))
 	{
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78622.c b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
new file mode 100644
index 0000000..44c9b0b
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
@@ -0,0 +1,39 @@
+/* PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value
+   incorrect with overflow/wrapping */
+/* { dg-additional-options "-Wformat-length=2" } */
+
+__attribute__((noinline, noclone)) int
+foo (int x)
+{
+  if (x < 4096 + 8 || x >= 4096 + 256 + 8)
+    return -1;
+  char buf[5];
+  return __builtin_sprintf (buf, "%hhd", x + 1);
+}
+
+int
+bar (unsigned int x)
+{
+  if (x < 64 || x > 2U * __INT_MAX__ - 10)
+    return -1;
+  char buf[1];
+  return __builtin_sprintf (buf, "%d", x + 1);   /* { dg-warn "directive writing between 1 and 11 bytes into a region of size 1" "int32plus" { target { int32plus } } */
+}
+
+int
+main ()
+{
+  if (__SCHAR_MAX__ != 127 || __CHAR_BIT__ != 8 || __SIZEOF_INT__ != 4)
+    return 0;
+  if (foo (4095 + 9) != 1
+      || foo (4095 + 32) != 2
+      || foo (4095 + 127) != 3
+      || foo (4095 + 128) != 4
+      || foo (4095 + 240) != 3
+      || foo (4095 + 248) != 2
+      || foo (4095 + 255) != 2
+      || foo (4095 + 256) != 1)
+    __builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
index 4dddccdf..260f4fc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
@@ -25,6 +25,9 @@ char buf8k [8192];
 #define concat(a, b)   a ## b
 #define CAT(a, b)      concat (a, b)
 
+/* Calls to this function must not be eliminated.  */
+void must_not_eliminate (void);
+
 #define EQL(expect, size, fmt, ...)					\
   void __attribute__ ((noinline, noclone))				\
   CAT (test_on_line_, __LINE__)(void)					\
@@ -34,7 +37,7 @@ char buf8k [8192];
 	char *dst = size < 0 ? buf : buf8k + sizeof buf8k - size;	\
 	int result = __builtin_sprintf (dst, fmt, __VA_ARGS__);		\
 	if (result != expect)						\
-	  __builtin_abort ();						\
+	  must_not_eliminate ();					\
       }									\
   }
 
@@ -50,7 +53,7 @@ char buf8k [8192];
 	char *dst = size < 0 ? buf : buf8k + sizeof buf8k - size;	\
 	int result = __builtin_sprintf (dst, fmt, __VA_ARGS__);		\
 	if (result < min || max < result)				\
-	  __builtin_abort ();						\
+	  must_not_eliminate ();					\
       }									\
   }
 
@@ -75,6 +78,8 @@ EQL (0, 0, "%-s", "");
 EQL (1, 1, "%c",  'x');
 EQL (1, 1, "%-s", "x");
 
+EQL (1, 2, "%c",  'x');
+
 EQL (4, 4, "%4c", 'x');
 
 /* Verify that exceeding the environmental limit of 4095 bytes for
@@ -179,7 +184,7 @@ RNG (4,  4, 32, "%zu", sz)
 
 /* Exercise bug 78586.  */
 RNG (4,  4, 32, "%lu", (unsigned long)i)
-RNG (4,  4, 32, "%lu", (unsigned)u)
+RNG (4,  4, 32, "%lu", (unsigned long)u)
 RNG (4,  4, 32, "%lu", (unsigned long)li)
 RNG (4,  4, 32, "%lu", (unsigned long)lu)
 RNG (4,  4, 32, "%lu", (unsigned long)sz)
@@ -259,21 +264,24 @@ RNG (0,  6,   8, "%s%ls", "1", L"2");
 /* Verify that no call to abort has been eliminated and that each call
    is at the beginning of a basic block (and thus the result of a branch).
    This latter test tries to verify that the test preceding the call to
-   abort has not been eliminated either.
+   the must_not_eliminate() function has not been eliminated either.
 
    The expected output looks something like this:
 
    <bb 2>:
    result_3 = __builtin_sprintf (&MEM[(void *)&buf8k + 8192B], "%c", 32);
    if (result_3 != 0)
-     goto <bb 3>;
+     goto <bb 3>; [50.0%]
    else
-     goto <bb 4>;
+     goto <bb 4>; [50.0%]
 
-   <bb 3>:
-   __builtin_abort ();
+   <bb 3>[50.0%]:
+   must_not_eliminate ();
 
 */
 
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 124 "optimized" { target { ilp32 || lp64 } } } } */
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } } */
+/*  Only conditional calls to abort should be made (with any probability):
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *must_not_eliminate" 124 "optimized" { target { ilp32 || lp64 } } } }
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *must_not_eliminate" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } }
+    No unconditional calls to abort should be made:
+    { dg-final { scan-tree-dump-not ";\n *must_not_eliminate" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
index dbb0dd9..c4489ac 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
@@ -8,18 +8,20 @@
 #define FAIL(line)  CAT (failure_on_line_, line)
 #define PASS(line)  CAT (success_on_line_, line)
 
-/* Emit a call to a function named failure_on_line_NNN when EXPR is false.  */
-#define ASSERT(value, expect)			\
+/* Emit a call to a function named failure_on_line_NNN when VALUE
+   is not equal to the constant EXPECTED, otherwise emit a call to
+   function success_on_line_NNN.  */
+#define ASSERT(value, expected)			\
   do {						\
     extern void FAIL (__LINE__)(int);		\
     extern void PASS (__LINE__)(int);		\
-    if (value == expect)			\
+    if (value == expected)			\
       PASS (__LINE__)(value);			\
     else					\
       FAIL (__LINE__)(value);			\
   } while (0)
 
-#define T(expect, ...)					\
+#define EQL(expect, ...)				\
   do {							\
     int n = __builtin_snprintf (0, 0, __VA_ARGS__);	\
     ASSERT (n, expect);					\
@@ -27,88 +29,118 @@
 
 int ival (int i) { return i; }
 
+/* Generate a signed int value in the specified range.  */
+
+static int
+int_range (int min, int max)
+{
+  extern int int_value (void);
+  int val = int_value ();
+  if (val < min || max < val)
+    val = min;
+  return val;
+}
+
+#define R(min, max) int_range (min, max)
+
 void test_arg_int (int i, int n)
 {
-  T (1, "%i", ival (0));
-  T (1, "%i", ival (1));
-  T (2, "%i%i", ival (0), ival (1));
-  T (3, "%i%i%i", ival (0), ival (1), ival (9));
-  T (5, "%i %i %i", ival (0), ival (1), ival (9));
+  EQL (1, "%i", ival (0));
+  EQL (1, "%i", ival (1));
+  EQL (2, "%i%i", ival (0), ival (1));
+  EQL (3, "%i%i%i", ival (0), ival (1), ival (9));
+  EQL (5, "%i %i %i", ival (0), ival (1), ival (9));
 
-  T (5, "%i %i %i", ival (0), ival (1), ival (9));
+  EQL (5, "%i %i %i", ival (0), ival (1), ival (9));
 
-  T (13, "%hhu.%hhu.%hhu.%hhu", ival (23), ival (78), ival (216), ival (135));
+  EQL (13, "%hhu.%hhu.%hhu.%hhu", ival (23), ival (78), ival (216), ival (135));
 
   for (i = 0; i != 9; ++i)
-    T (1, "%i", i);
+    EQL (1, "%i", i);
 
   for (i = -n; i != n; ++i)
-    T (8, "%08x", i);
+    EQL (8, "%08x", i);
 
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
-  T (0, "%.0d", ival (0));
-  T (0, "%.0i", ival (0));
-  T (0, "%.0o", ival (0));
-  T (0, "%.0u", ival (0));
-  T (0, "%.0x", ival (0));
-
-  T (0, "%.*d", 0, ival (0));
-  T (0, "%.*i", 0, ival (0));
-  T (0, "%.*o", 0, ival (0));
-  T (0, "%.*u", 0, ival (0));
-  T (0, "%.*x", 0, ival (0));
-
-  T (1, "%1.0d", ival (0));
-  T (1, "%1.0i", ival (0));
-  T (1, "%1.0o", ival (0));
-  T (1, "%1.0u", ival (0));
-  T (1, "%1.0x", ival (0));
+  EQL (0, "%.0d", ival (0));
+  EQL (0, "%.0i", ival (0));
+  EQL (0, "%.0o", ival (0));
+  EQL (0, "%.0u", ival (0));
+  EQL (0, "%.0x", ival (0));
+
+  EQL (0, "%.*d", 0, ival (0));
+  EQL (0, "%.*i", 0, ival (0));
+  EQL (0, "%.*o", 0, ival (0));
+  EQL (0, "%.*u", 0, ival (0));
+  EQL (0, "%.*x", 0, ival (0));
+
+  EQL (1, "%1.0d", ival (0));
+  EQL (1, "%1.0i", ival (0));
+  EQL (1, "%1.0o", ival (0));
+  EQL (1, "%1.0u", ival (0));
+  EQL (1, "%1.0x", ival (0));
+
+  EQL (4, "%hhi", R (-128, -127));
+  EQL (3, "%hhi", R ( -99,  -10));
+  EQL (2, "%hhi", R (  -9,   -1));
+  EQL (1, "%hhi", R (   0,    9));
+  EQL (1, "%hhi", R (   0,    9));
+
+  EQL (1, "%1.0hhi", R (   0,    1));
+  EQL (1, "%1.1hhi", R (   0,    9));
+  EQL (2, "%1.2hhi", R (   0,    9));
+  EQL (3, "%1.3hhi", R (   0,    9));
+
+  EQL (1, "%hhi", R (1024, 1033));
+  EQL (2, "%hhi", R (1034, 1123));
+  EQL (1, "%hhu", R (1024, 1033));
+  EQL (2, "%hhu", R (1034, 1123));
 }
 
 void test_arg_string (const char *s)
 {
-  T ( 0, "%-s", "");
-  T ( 1, "%%");
-  T ( 1, "%-s", "1");
-  T ( 2, "%-s", "12");
-  T ( 3, "%-s", "123");
-  T ( 5, "s=%s", "123");
-  T (10, "%%s=\"%s\"", "12345");
-
-  T ( 1, "%.*s", 1, "123");
-  T ( 2, "%.*s", 2, "123");
-  T ( 3, "%.*s", 3, "123");
-  T ( 3, "%.*s", 4, "123");
-
-  T ( 1, "%1.*s", 1, "123");
-  T ( 2, "%1.*s", 2, "123");
-  T ( 3, "%1.*s", 3, "123");
-  T ( 3, "%1.*s", 4, "123");
-  T ( 4, "%4.*s", 1, "123");
-  T ( 4, "%4.*s", 2, "123");
-  T ( 4, "%4.*s", 3, "123");
-  T ( 4, "%4.*s", 4, "123");
-  T ( 4, "%4.*s", 5, "123");
+  EQL ( 0, "%-s", "");
+  EQL ( 1, "%%");
+  EQL ( 1, "%-s", "1");
+  EQL ( 2, "%-s", "12");
+  EQL ( 3, "%-s", "123");
+  EQL ( 5, "s=%s", "123");
+  EQL (10, "%%s=\"%s\"", "12345");
+
+  EQL ( 1, "%.*s", 1, "123");
+  EQL ( 2, "%.*s", 2, "123");
+  EQL ( 3, "%.*s", 3, "123");
+  EQL ( 3, "%.*s", 4, "123");
+
+  EQL ( 1, "%1.*s", 1, "123");
+  EQL ( 2, "%1.*s", 2, "123");
+  EQL ( 3, "%1.*s", 3, "123");
+  EQL ( 3, "%1.*s", 4, "123");
+  EQL ( 4, "%4.*s", 1, "123");
+  EQL ( 4, "%4.*s", 2, "123");
+  EQL ( 4, "%4.*s", 3, "123");
+  EQL ( 4, "%4.*s", 4, "123");
+  EQL ( 4, "%4.*s", 5, "123");
 
   const char *a = "123";
   const char *b = "456";
 
-  T ( 3, "%-s", s ? a : b);
-  T ( 0, "%.0s", s);
-  T ( 1, "%1.1s", s);
-  T ( 2, "%2.2s", s);
-  T ( 2, "%2.1s", s);
+  EQL ( 3, "%-s", s ? a : b);
+  EQL ( 0, "%.0s", s);
+  EQL ( 1, "%1.1s", s);
+  EQL ( 2, "%2.2s", s);
+  EQL ( 2, "%2.1s", s);
 }
 
 void test_arg_multiarg (int i, double d)
 {
-  T (16, "%i %f %s", 123, 3.14, "abc");
-  T (16, "%12i %s", i, "abc");
-  T (16, "%*i %s", 12, i, "abc");
+  EQL (16, "%i %f %s", 123, 3.14, "abc");
+  EQL (16, "%12i %s", i, "abc");
+  EQL (16, "%*i %s", 12, i, "abc");
 }
 
-#define TV(expect, fmt, va)				\
+#define EQLv(expect, fmt, va)				\
   do {							\
     int n = __builtin_vsnprintf (0, 0, fmt, va);	\
     ASSERT (n, expect);					\
@@ -116,21 +148,21 @@ void test_arg_multiarg (int i, double d)
 
 void test_va_int (__builtin_va_list va)
 {
-  TV ( 2, "%02hhx", va);
-  TV ( 2, "%02.*hhx", va);
-  TV ( 4, "%04hx", va);
-  TV ( 4, "%04.*hx", va);
+  EQLv ( 2, "%02hhx", va);
+  EQLv ( 2, "%02.*hhx", va);
+  EQLv ( 4, "%04hx", va);
+  EQLv ( 4, "%04.*hx", va);
 }
 
 void test_va_multiarg (__builtin_va_list va)
 {
-  TV ( 8, "%8x", va);
-  TV ( 8, "% 8x", va);
-  TV ( 9, "%9x", va);
-  TV (11, "%11o", va);
-  TV (12, "%12o", va);
+  EQLv ( 8, "%8x", va);
+  EQLv ( 8, "% 8x", va);
+  EQLv ( 9, "%9x", va);
+  EQLv (11, "%11o", va);
+  EQLv (12, "%12o", va);
 
-  TV (16, "%12i %3.2s", va);
+  EQLv (16, "%12i %3.2s", va);
 }
 
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
index 4c41234..abd49df 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
@@ -8,6 +8,8 @@
    { dg-options "-O2 -Wformat -fdump-tree-optimized" }
    { dg-require-effective-target int32plus } */
 
+typedef __SIZE_TYPE__ size_t;
+
 #define CONCAT(a, b) a ## b
 #define CAT(a, b)    CONCAT (a, b)
 
@@ -50,6 +52,19 @@ void test_arg_int (int width, int prec, int i, int n)
 
   T ("%i", R (1, 10));
 
+  /* Each of the bounds of the ranges below results in just one byte
+     on output because they convert to the value 9 in type char, yet
+     other values in those ranges can result in up to four bytes.
+     For example, 4240 converts to -112.  Verify that the output
+     isn't folded into a constant.  This assumes __CHAR_BIT__ of 8.  */
+  T ("%hhi", R (4104, 4360) + 1);
+  T ("%hhu", R (4104, 4360) + 1);
+
+  /* Here, the range includes all possible lengths of output for
+     a 16-bit short and 32-bit int.  */
+  T ("%hi", R (65536, 65536 * 2));
+  T ("%hu", R (65536, 65536 * 2));
+
   T ("%'i", 1234567);
 
   for (i = -n; i != n; ++i)
@@ -104,6 +119,7 @@ void test_invalid_directive (void)
   T ("abc%Q");    /* { dg-warning "unknown conversion type character .Q." } */
 }
 
+
 /* Use 'grep "^ *T (" builtin-sprintf-6.c  | wc -l' to determine
    the count for the directive below.
-   { dg-final { scan-tree-dump-times "snprintf" 42 "optimized"} } */
+   { dg-final { scan-tree-dump-times "snprintf" 46 "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
index fae584e..f49422a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -1039,8 +1039,8 @@ void test_sprintf_chk_s_nonconst (int w, int p, const char *s)
      is not.  */
   T ( 1, "%*s",  w, "");
   T ( 1, "%*s",  w, "1");       /* { dg-warning "nul past the end" } */
-  T ( 1, "%.*s", w, "");
-  T ( 1, "%.*s", w, "1");       /* { dg-warning "may write a terminating nul" } */
+  T ( 1, "%.*s", p, "");
+  T ( 1, "%.*s", p, "1");       /* { dg-warning "may write a terminating nul" } */
   T ( 1, "%.*s", w, "123");     /* { dg-warning "writing between 0 and 3 bytes into a region of size 1" } */
 
   T ( 1, "%*s", w, "123");      /* { dg-warning "writing 3 or more bytes into a region of size 1" } */
@@ -1294,6 +1294,12 @@ void test_sprintf_chk_int_nonconst (int w, int p, int a)
   T (3, "%2x",          a);
 
   T (1, "%.*d",      p, a);
+
+  T (4, "%i %i",        a, a);
+  /* The following will definitely be "writing a terminating nul past the end"
+     (i.e., not "may write".)  */
+  T (4, "%i %i ",       a, a);      /* { dg-warning "writing a terminating nul past the end" } */
+  T (4, "%i %i %i",     a, a, a);   /* { dg-warning "into a region" }*/
 }
 
 void test_sprintf_chk_e_nonconst (int w, int p, double d)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
index 0cb02b7..121ed4e 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
@@ -2,15 +2,18 @@
    Test to verify that the correct range information is made available to the
    -Wformat-lenght check to prevent warnings.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wformat -Wformat-length" } */
+/* { dg-options "-O2 -Wformat -Wformat-length -fdump-tree-optimized" } */
 
+void abort (void);
 int snprintf (char*, __SIZE_TYPE__, const char*, ...);
 
 void fuchar (unsigned char j, char *p)
 {
   if (j > 99)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fschar (signed char j, char *p)
@@ -20,14 +23,17 @@ void fschar (signed char j, char *p)
   if (k > 99)
     return;
 
-  snprintf (p, 3, "%3hhu", k);   /* { dg-bogus "" "unsigned char" { xfail *-*-* } } */
+  if (3 != snprintf (p, 4, "%3hhu", k))
+    abort ();
 }
 
 void fushrt (unsigned short j, char *p)
 {
   if (j > 999)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fshrt (short j, char *p)
@@ -37,7 +43,8 @@ void fshrt (short j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3hu", k);
+  if (3 != snprintf (p, 4, "%3hu", k))
+    abort ();
 }
 
 void fuint (unsigned j, char *p)
@@ -54,13 +61,16 @@ void fint (int j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3u", k);   /* { dg-bogus "" "unsigned int" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3u", k);
 }
 
 void fulong (unsigned long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3lu", j);
 }
 
@@ -71,13 +81,16 @@ void flong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3lu", k);   /* { dg-bogus "" "unsigned long" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3lu", k);
 }
 
 void fullong (unsigned long long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3llu", j);
 }
 
@@ -88,5 +101,7 @@ void fllong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3llu", k);   /* { dg-bogus "" "unsigned long long" { xfail lp64 } } */
+  snprintf (p, 4, "%3llu", k);
 }
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
index 916df79..35a5bd0 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
@@ -166,27 +166,48 @@ test_c (char c)
   EQL (5,  6, "%c %c %c",  c,   c,   c);
 }
 
-/* Generate a pseudo-random value in the specified range.  The return
-   value must be unsigned char to work around limitations in the GCC
-   range information.  Similarly for the declaration of rand() whose
-   correct return value should be int, but that also prevents the range
-   information from making it to the printf pass.  */
+/* Generate a pseudo-random unsigned value.  */
 
-unsigned char uchar_range (unsigned min, unsigned max)
+unsigned __attribute__ ((noclone, noinline))
+unsigned_value (void)
 {
-  extern unsigned rand (void);
+  extern int rand ();
+  return rand ();
+}
 
-  unsigned x;
-  x = rand ();
+/* Generate a pseudo-random signed value.  */
 
-  if (x < min)
-    x = min;
-  else if (max < x)
-    x = max;
+int __attribute__ ((noclone, noinline))
+int_value (void)
+{
+  extern int rand ();
+  return rand ();
+}
+
+/* Generate an unsigned char value in the specified range.  */
 
+static unsigned char
+uchar_range (unsigned min, unsigned max)
+{
+  unsigned x = unsigned_value ();
+  if (x < min || max < x)
+    x = min;
   return x;
 }
 
+/* Generate a signed int value in the specified range.  */
+
+static int
+int_range (int min, int max)
+{
+  int val = int_value ();
+  if (val < min || max < val)
+    val = min;
+  return val;
+}
+
+#define IR(min, max) int_range (min, max)
+
 static void __attribute__ ((noinline, noclone))
 test_d_i (int i, long li)
 {
@@ -266,9 +287,35 @@ test_d_i (int i, long li)
   RNG ( 1,  4,  5, "%hhi",     i);
   RNG ( 1,  3,  4, "%hhu",     i);
 
+  RNG ( 3,  4,  5, "%hhi",     IR (-128,  -10));
+  RNG ( 2,  4,  5, "%hhi",     IR (-128,   -1));
+  RNG ( 1,  4,  5, "%hhi",     IR (-128,    0));
+
+  RNG ( 1,  4,  5, "%1hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%2hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%3hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%4hhi",    IR (-128,    0));
+  RNG ( 1,  5,  6, "%5hhi",    IR (-128,    0));
+  RNG ( 1,  6,  7, "%6hhi",    IR (-128,    0));
+  RNG ( 2,  6,  7, "%6hhi",    IR (-128,   10));
+
+  RNG ( 0,  1,  2, "%.hhi",    IR (   0,    1));
+  RNG ( 0,  1,  2, "%.0hhi",   IR (   0,    1));
+  RNG ( 0,  1,  2, "%0.0hhi",  IR (   0,    1));   /* { dg-warning ".0. flag ignored with precision" } */
+  RNG ( 0,  1,  2, "%*.0hhi",  0, IR (   0,    1));
+
+  RNG ( 1,  2,  3, "%hhi",     IR (1024, 1034));
+  RNG ( 1,  4,  5, "%hhi",     IR (1024, 2048));
+  RNG ( 2,  3,  4, "%hhi",     IR (1034, 1151));
+
+  RNG ( 1,  2,  3, "%hhu",     IR (1024, 1034));
+  RNG ( 1,  3,  4, "%hhu",     IR (1024, 2048));
+  RNG ( 2,  3,  4, "%hhu",     IR (1034, 1151));
+
 #if __SIZEOF_SHORT__ == 2
   RNG ( 1,  6,  7, "%hi",      i);
   RNG ( 1,  5,  6, "%hu",      i);
+
 #elif __SIZEOF_SHORT__ == 4
   RNG ( 1, 11, 12, "%hi",      i);
   RNG ( 1, 10, 11, "%hu",      i);

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jeff Law]

On 12/01/2016 07:31 PM, Martin Sebor wrote:
> On 12/01/2016 02:15 AM, Jakub Jelinek wrote:
>> On Thu, Dec 01, 2016 at 08:26:47AM +0100, Jakub Jelinek wrote:
>>> Isn't this too simplistic?  I mean, if you have say dirtype of signed
>>> char
>>> and argmin say 4096 + 32 and argmax say 4096 + 64, (signed char) arg
>>> has range 32, 64, while I think your routine will yield -128, 127 (well,
>>> 0 as min and -128 as max as that is what you return for signed type).
>>>
>>> Can't you subtract argmax - argmin (best just in wide_int, no need to
>>> build
>>> trees), and use what you have just for the case where that number
>>> doesn't
>>> fit into the narrower precision, otherwise if argmin - (dirtype) argmin
>>> == argmax - (dirtype) argmax, just use (dirtype) argmin and (dirtype)
>>> argmax
>>> as the range, and in case that it crosses a boundary figure if you
>>> can do
>>> anything than the above?  Guess all cases of signed/unsigned dirtype
>>> and/or
>>> argtype need to be considered.
>>
>> Richard noted that you could have a look at CONVERT_EXPR_CODE_P
>> handling in extract_range_from_unary_expr.  I think it is the
>>               || (vr0.type == VR_RANGE
>>                   && integer_zerop (int_const_binop (RSHIFT_EXPR,
>>                        int_const_binop (MINUS_EXPR, vr0.max, vr0.min),
>>                          size_int (TYPE_PRECISION (outer_type)))))))
>> part that is important here for the narrowing conversion.
>
> Thanks, that was a helpful suggestion!  Attached is an update that
> follows the vrp approach.  I assume the infinity stuff is specific
> to the VRP pass and not something I need to worry about here.
>
> Martin
>
> PS To your earlier question, argmin and argmax have the same meaning
> in the added helper function as in its caller.
>
> gcc-78622.diff
>
>
> PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping
>
> gcc/ChangeLog:
>
> 	PR middle-end/78622
> 	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
> 	rather than res.bounded.
> 	(adjust_range_for_overflow): New function.
> 	(format_integer): Always set res.bounded to true unless either
> 	precision or width is specified and unknown.
> 	Call adjust_range_for_overflow.
> 	(format_directive): Remove vestigial quoting.
> 	(add_bytes): Correct the computation of boundrange used to
> 	decide whether a warning is of a "maybe" or "defnitely" kind.
s/defnitely/definitely/

>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/78622
> 	* gcc.c-torture/execute/pr78622.c: New test.
> 	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
> 	behavior inadvertently introduced in a previous commit.  Tighten
> 	up final checking.
> 	* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
> 	Add test cases.
> 	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
> 	add a final optimization check.
> 	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.


>
> diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
> index 99a635a..95a8710 100644
> --- a/gcc/gimple-ssa-sprintf.c
> +++ b/gcc/gimple-ssa-sprintf.c
> @@ -795,6 +795,59 @@ get_width_and_precision (const conversion_spec &spec,
>    *pprec = prec;
>  }
>
> +/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
> +   argument, due to the conversion from either *ARGMIN or *ARGMAX to
> +   the type of the directive's formal argument it's possible for both
> +   to result in the same number of bytes or a range of bytes that's
> +   less than the number of bytes that would result from formatting
> +   some other value in the range [*ARGMIN, *ARGMAX].  This can be
> +   determined by checking for the actual argument being in the range
> +   of the type of the directive.  If it isn't it must be assumed to
> +   take on the full range of the directive's type.
> +   Return true when the range has been adjusted to the range of
> +   DIRTYPE, false otherwise.  */
I wish I'd counted how many times I read that.

> +
> +static bool
> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
> +{
> +  unsigned argprec = TYPE_PRECISION (TREE_TYPE (*argmin));
> +  unsigned dirprec = TYPE_PRECISION (dirtype);
> +
> +  /* The logic here was inspired/lifted from  the CONVERT_EXPR_CODE_P
> +     branch in the extract_range_from_unary_expr function in tree-vrp.c.
> +  */
Formatting it.  If the comment-close won't fit, then line wrap prior to 
the last word in the comment.


> +
> +  if (TREE_CODE (*argmin) == INTEGER_CST
> +      && TREE_CODE (*argmax) == INTEGER_CST
> +      && (dirprec >= argprec
> +	  || integer_zerop (int_const_binop (RSHIFT_EXPR,
> +					     int_const_binop (MINUS_EXPR,
> +							      *argmax,
> +							      *argmin),
> +					     size_int (dirprec)))))
> +  {
> +    *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0, false);
> +    *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0, false);
> +    return false;
> +  }
So in this case we're not changing the values, we're just converting 
them to a equal or wider type, right?  Thus no adjustment (what about a 
sign change?  Is that an adjustment?) and we return false per the 
function's specifications.

What about overflows when either argmin or argmax won't fit into dirtype 
without an overflow?  What's the right behavior there?

> +
> +  tree dirmin = TYPE_MIN_VALUE (dirtype);
> +  tree dirmax = TYPE_MAX_VALUE (dirtype);
 From this point onward argmin/argmax are either not integers or we're 
doing a narrowing conversion, right?  In both cases the fact that we're 
doing a narrowing conversion constrains the range

> +
> +  if (TYPE_UNSIGNED (dirtype))
> +    {
> +      *argmin = dirmin;
> +      *argmax = dirmax;
> +    }
> +  else
> +    {
> +      *argmin = integer_zero_node;
> +      *argmax = dirmin;
> +    }
Should this be dirmax? Am I missing something here?


Jeff

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Martin Sebor]

Thanks for looking at this!  I realize it's dense code and not easy
to make sense out of.

>> PR middle-end/78622 - [7 Regression]
>> -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping
>>
>> gcc/ChangeLog:
>>
>>     PR middle-end/78622
>>     * gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
>>     rather than res.bounded.
>>     (adjust_range_for_overflow): New function.
>>     (format_integer): Always set res.bounded to true unless either
>>     precision or width is specified and unknown.
>>     Call adjust_range_for_overflow.
>>     (format_directive): Remove vestigial quoting.
>>     (add_bytes): Correct the computation of boundrange used to
>>     decide whether a warning is of a "maybe" or "defnitely" kind.
> s/defnitely/definitely/

Will fix.

>> +/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
>> +   argument, due to the conversion from either *ARGMIN or *ARGMAX to
>> +   the type of the directive's formal argument it's possible for both
>> +   to result in the same number of bytes or a range of bytes that's
>> +   less than the number of bytes that would result from formatting
>> +   some other value in the range [*ARGMIN, *ARGMAX].  This can be
>> +   determined by checking for the actual argument being in the range
>> +   of the type of the directive.  If it isn't it must be assumed to
>> +   take on the full range of the directive's type.
>> +   Return true when the range has been adjusted to the range of
>> +   DIRTYPE, false otherwise.  */
> I wish I'd counted how many times I read that.

Let me see if I can word it better.

>
>> +
>> +static bool
>> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
>> +{
>> +  unsigned argprec = TYPE_PRECISION (TREE_TYPE (*argmin));
>> +  unsigned dirprec = TYPE_PRECISION (dirtype);
>> +
>> +  /* The logic here was inspired/lifted from  the CONVERT_EXPR_CODE_P
>> +     branch in the extract_range_from_unary_expr function in tree-vrp.c.
>> +  */
> Formatting it.  If the comment-close won't fit, then line wrap prior to
> the last word in the comment.

Okay.  I didn't remember what the exact rules were.

>> +
>> +  if (TREE_CODE (*argmin) == INTEGER_CST
>> +      && TREE_CODE (*argmax) == INTEGER_CST
>> +      && (dirprec >= argprec
>> +      || integer_zerop (int_const_binop (RSHIFT_EXPR,
>> +                         int_const_binop (MINUS_EXPR,
>> +                                  *argmax,
>> +                                  *argmin),
>> +                         size_int (dirprec)))))
>> +  {
>> +    *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0,
>> false);
>> +    *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0,
>> false);
>> +    return false;
>> +  }
> So in this case we're not changing the values, we're just converting
> them to a equal or wider type, right?  Thus no adjustment (what about a
> sign change?  Is that an adjustment?) and we return false per the
> function's specifications.

This casts the value to the destination type, so it may result in
different values.  The important postcondition it preserves is that
the difference ARGMAX - ARGMIN is less than or equal to TYPE_MAX of
the directive's unsigned TYPE.  (If it isn't, the range cannot be
converted.)  This lets us take, say:

   int f (int i)
   {
     if (i < 1024 || 1033 < i)
        i = 1024;

     return snprintf (0, 0, "%hhi", i);
   }

and fold the function call into 1 because (signed char)1024 is 0 and
(signed char)1033 is 9, and every other value in that same original
range is also in the same range after the conversion.  We know it's
safe because (1033 - 1024 <= UCHAR_MAX) holds.  But in in this:

   int g (int i)
   {
     if (i < 1024 || 1289 < i)
        i = 1024;

     return snprintf (0, 0, "%hhi", i);
   }

even though (signed char)1024 is 0 and (signed char)1289 is also 9
like above, since (1289 - 1024) is 265 and thus greater than UCHAR_MAX,
folding the result wouldn't be safe (for example, (sighed char)1034
yields 10).

I picture this as a window bounded by the range of the directive's
type sliding within another window bounded by the argument's type:

   [<-----[...dirtype...]--->]

the outer brackets delimit the range of the argument and the inner
ones the directive's.  The smaller window can slide left and right
and it can shrink but it can't be wider that the directive's type's
range.

> What about overflows when either argmin or argmax won't fit into dirtype
> without an overflow?  What's the right behavior there?

That's fine just as long as the property above holds.

I think the algorithm works but the code came from tree-vrp where
there are all sorts of additional checks for some infinities which
VRP distinguishes from type limits like SCHAR_MIN and _MAX.  I don't
fully understand those and so I'd feel better if someone who does
double checked this code.

>
>> +
>> +  tree dirmin = TYPE_MIN_VALUE (dirtype);
>> +  tree dirmax = TYPE_MAX_VALUE (dirtype);
> From this point onward argmin/argmax are either not integers or we're
> doing a narrowing conversion, right?  In both cases the fact that we're
> doing a narrowing conversion constrains the range

Argmin and argmax are always integers.  The rest of the function
handles the case where the postcondition above doesn't hold (e.g.,
in function g above).

>
>> +
>> +  if (TYPE_UNSIGNED (dirtype))
>> +    {
>> +      *argmin = dirmin;
>> +      *argmax = dirmax;
>> +    }
>> +  else
>> +    {
>> +      *argmin = integer_zero_node;
>> +      *argmax = dirmin;
>> +    }
> Should this be dirmax? Am I missing something here?

It's dirmin because for a signed type, due to the sign, it results
in more bytes on output than dirmin would.  (E.g., with SCHAR_MIN
= -128 and SCHAR_MAX = 127 it's four bytes vs three.)

Martin

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jakub Jelinek]

Hi!

On Thu, Dec 01, 2016 at 07:31:18PM -0700, Martin Sebor wrote:
> +static bool
> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
> +{
> +  if (TYPE_UNSIGNED (dirtype))
> +    {
> +      *argmin = dirmin;
> +      *argmax = dirmax;
> +    }
> +  else
> +    {
> +      *argmin = integer_zero_node;
> +      *argmax = dirmin;
> +    }

I still don't really like this mixing of ranges of values and picking of values
which result in shortest and longest representation, it is confusing and
will be a maintainance nightmare.

IMHO much cleaner is first figure out the range the argument (in argtype)
has.  I.e. look at VR_RANGE and if it is missing, perhaps find out another
argtype and in any case, use TYPE_{MIN,MAX}_VALUE (argtype) as the range.
I think that should probably be the range presented to the user in
diagnostics (i.e. res.arg{min,max}).

Next step is to adjust this range for the case where dirtype is different
from argtype.  This should be done regardless of what way you get the first
range from (whether from VR_RANGE or VR_VARYING etc.).  The result of this
still should be a range of values in dirtype.

And the last step should be to pick the values from that range which
has shortest and longest representation.  For unsigned dirtype
that are the bounds of the range from earlier step, for signed dirtype
something different (if both bounds are >= 0, then also just those bounds,
if both bounds are < 0, then the bounds swapped, otherwise 0 as minimum,
then e.g. try both bounds what has longer representation, or take some short
path e.g. if abs of the negative bound is >= the positive bound, then use
the negative bound as longest, otherwise try both).

	Jakub

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 4402 bytes --]

On 12/05/2016 01:26 PM, Jakub Jelinek wrote:
> Hi!
>
> On Thu, Dec 01, 2016 at 07:31:18PM -0700, Martin Sebor wrote:
>> +static bool
>> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
>> +{
>> +  if (TYPE_UNSIGNED (dirtype))
>> +    {
>> +      *argmin = dirmin;
>> +      *argmax = dirmax;
>> +    }
>> +  else
>> +    {
>> +      *argmin = integer_zero_node;
>> +      *argmax = dirmin;
>> +    }
>
> I still don't really like this mixing of ranges of values and picking of values
> which result in shortest and longest representation, it is confusing and
> will be a maintainance nightmare.
>
> IMHO much cleaner is first figure out the range the argument (in argtype)
> has.  I.e. look at VR_RANGE and if it is missing, perhaps find out another
> argtype and in any case, use TYPE_{MIN,MAX}_VALUE (argtype) as the range.
> I think that should probably be the range presented to the user in
> diagnostics (i.e. res.arg{min,max}).
>
> Next step is to adjust this range for the case where dirtype is different
> from argtype.  This should be done regardless of what way you get the first
> range from (whether from VR_RANGE or VR_VARYING etc.).  The result of this
> still should be a range of values in dirtype.
>
> And the last step should be to pick the values from that range which
> has shortest and longest representation.  For unsigned dirtype
> that are the bounds of the range from earlier step, for signed dirtype
> something different (if both bounds are >= 0, then also just those bounds,
> if both bounds are < 0, then the bounds swapped, otherwise 0 as minimum,
> then e.g. try both bounds what has longer representation, or take some short
> path e.g. if abs of the negative bound is >= the positive bound, then use
> the negative bound as longest, otherwise try both).

I take this as your confirmation that the function does do the right
thing.  If not, or if you can't confirm that, it would be helpful if
you could let me know so that I can ask Richard to look it over.

The pass already does pretty much what you describe but I was able
to simplify the format_integer function somewhat and also arrange
for the informational note mentioning the range of argument values
to more closely reflect what I think you suggest.

With the attached patch the following example produces the output
below.  In the first function, because the pass uses the actual
argument range unchanged, the warning prints the exact byte count
and the note the original range.  In the second function, the range
must be adjusted to that of the directive's type, and the warning
prints a range of bytes and the note says "using the range [0, -128]"
to indicate that it used a different range than the range of
the actual argument.  It might be possible to change the note to
say something like "using the range of type 'signed char' or
something like that, to make it even clearer that the whole range
of the type is being used.  But these further tweaks should be made
independently of this patch, perhaps as part bug 77696 if and when
I get to it.

Martin

$ cat b.c && /build/gcc-78622/gcc/xgcc -B /build/gcc-78622/gcc -O2 -S 
-Wall -Wextra -Wpedantic b.c
char d[1];

void f (int i)
{
   if (i < 1024 || 1033 < i) i = 1024;

   __builtin_sprintf (d + 1, "%hhi", i);
}

void g (int i)
{
   if (i < 1024 || 3456 < i) i = 1024;

   __builtin_sprintf (d + 1, "%hhi", i);
}

b.c: In function ‘f’:
b.c:7:30: warning: ‘%hhi’ directive writing 1 byte into a region of size 
0 [-Wformat-length=]
    __builtin_sprintf (d + 1, "%hhi", i);
                               ^~~~
b.c:7:29: note: directive argument in the range [1024, 1033]
    __builtin_sprintf (d + 1, "%hhi", i);
                              ^~~~~~
b.c:7:3: note: format output 2 bytes into a destination of size 0
    __builtin_sprintf (d + 1, "%hhi", i);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
b.c: In function ‘g’:
b.c:14:30: warning: ‘%hhi’ directive writing between 1 and 4 bytes into 
a region of size 0 [-Wformat-length=]
    __builtin_sprintf (d + 1, "%hhi", i);
                               ^~~~
b.c:14:29: note: using the range [0, -128] for directive argument
    __builtin_sprintf (d + 1, "%hhi", i);
                              ^~~~~~
b.c:14:3: note: format output between 2 and 5 bytes into a destination 
of size 0
    __builtin_sprintf (d + 1, "%hhi", i);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


[-- Attachment #2: gcc-78622.diff --]
[-- Type: text/x-patch, Size: 43881 bytes --]

PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping

gcc/ChangeLog:

	PR middle-end/78622
	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
	rather than res.bounded.
	(adjust_range_for_overflow): New function.
	(format_integer): Always set res.bounded to true unless either
	precision or width is specified and unknown.
	Call adjust_range_for_overflow.
	(format_directive): Remove vestigial quoting.  Always inform of
	argument value or range when it's available.
	(add_bytes): Correct the computation of boundrange used to
	decide whether a warning is of a "maybe" or "defnitely" kind.

gcc/testsuite/ChangeLog:

	PR middle-end/78622
	* gcc.c-torture/execute/pr78622.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
	behavior inadvertently introduced in a previous commit.  Tighten
	up final checking.
	* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
	Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
	add a final optimization check.
	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 99a635a..a888f55 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -637,7 +637,7 @@ min_bytes_remaining (unsigned HOST_WIDE_INT navail, const format_result &res)
   if (HOST_WIDE_INT_MAX <= navail)
     return navail;
 
-  if (1 < warn_format_length || res.bounded)
+  if (1 < warn_format_length || res.knownrange)
     {
       /* At level 2, or when all directives output an exact number
 	 of bytes or when their arguments were bounded by known
@@ -795,6 +795,59 @@ get_width_and_precision (const conversion_spec &spec,
   *pprec = prec;
 }
 
+/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
+   argument, due to the conversion from either *ARGMIN or *ARGMAX to
+   the type of the directive's formal argument it's possible for both
+   to result in the same number of bytes or a range of bytes that's
+   less than the number of bytes that would result from formatting
+   some other value in the range [*ARGMIN, *ARGMAX].  This can be
+   determined by checking for the actual argument being in the range
+   of the type of the directive.  If it isn't it must be assumed to
+   take on the full range of the directive's type.
+   Return true when the range has been adjusted to the range of
+   DIRTYPE, false otherwise.  */
+
+static bool
+adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
+{
+  unsigned argprec = TYPE_PRECISION (TREE_TYPE (*argmin));
+  unsigned dirprec = TYPE_PRECISION (dirtype);
+
+  /* The logic here was inspired/lifted from  the CONVERT_EXPR_CODE_P
+     branch in the extract_range_from_unary_expr function in tree-vrp.c.
+  */
+
+  if (TREE_CODE (*argmin) == INTEGER_CST
+      && TREE_CODE (*argmax) == INTEGER_CST
+      && (dirprec >= argprec
+	  || integer_zerop (int_const_binop (RSHIFT_EXPR,
+					     int_const_binop (MINUS_EXPR,
+							      *argmax,
+							      *argmin),
+					     size_int (dirprec)))))
+    {
+      *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0, false);
+      *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0, false);
+      return false;
+    }
+
+  tree dirmin = TYPE_MIN_VALUE (dirtype);
+  tree dirmax = TYPE_MAX_VALUE (dirtype);
+
+  if (TYPE_UNSIGNED (dirtype))
+    {
+      *argmin = dirmin;
+      *argmax = dirmax;
+    }
+  else
+    {
+      *argmin = integer_zero_node;
+      *argmax = dirmin;
+    }
+
+  return true;
+}
+
 /* Return a range representing the minimum and maximum number of bytes
    that the conversion specification SPEC will write on output for the
    integer argument ARG when non-null.  ARG may be null (for vararg
@@ -989,6 +1042,10 @@ format_integer (const conversion_spec &spec, tree arg)
 
   fmtresult res;
 
+  /* The result is bounded unless width or precision has been specified
+     whose value is unknown.  */
+  res.bounded = width != HOST_WIDE_INT_MIN && prec != HOST_WIDE_INT_MIN;
+
   /* Using either the range the non-constant argument is in, or its
      type (either "formal" or actual), create a range of values that
      constrain the length of output given the warning level.  */
@@ -1005,38 +1062,19 @@ format_integer (const conversion_spec &spec, tree arg)
       enum value_range_type range_type = get_range_info (arg, &min, &max);
       if (range_type == VR_RANGE)
 	{
-	  res.argmin = build_int_cst (argtype, wi::fits_uhwi_p (min)
-				      ? min.to_uhwi () : min.to_shwi ());
-	  res.argmax = build_int_cst (argtype, wi::fits_uhwi_p (max)
-				      ? max.to_uhwi () : max.to_shwi ());
-
-	  /* For a range with a negative lower bound and a non-negative
-	     upper bound, use one to determine the minimum number of bytes
-	     on output and whichever of the two bounds that results in
-	     the greater number of bytes on output for the upper bound.
-	     For example, for ARG in the range of [-3, 123], use 123 as
-	     the upper bound for %i but -3 for %u.  */
-	  if (wi::neg_p (min) && !wi::neg_p (max))
-	    {
-	      argmin = res.argmin;
-	      argmax = res.argmax;
-	      int minbytes = format_integer (spec, res.argmin).range.min;
-	      int maxbytes = format_integer (spec, res.argmax).range.max;
-	      if (maxbytes < minbytes)
-		argmax = res.argmin;
-
-	      argmin = integer_zero_node;
-	    }
-	  else
-	    {
-	      argmin = res.argmin;
-	      argmax = res.argmax;
-	    }
-
-	  /* The argument is bounded by the known range of values
-	     determined by Value Range Propagation.  */
-	  res.bounded = true;
-	  res.knownrange = true;
+	  argmin = build_int_cst (argtype, wi::fits_uhwi_p (min)
+				  ? min.to_uhwi () : min.to_shwi ());
+	  argmax = build_int_cst (argtype, wi::fits_uhwi_p (max)
+				  ? max.to_uhwi () : max.to_shwi ());
+
+	  /* Set KNOWNRANGE if the argument is in a known subrange
+	     of the directive's type (KNOWNRANGE may be reset below).  */
+	  res.knownrange
+	    = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), argmin)
+	       || !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), argmax));
+
+	  res.argmin = argmin;
+	  res.argmax = argmax;
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
@@ -1101,6 +1139,17 @@ format_integer (const conversion_spec &spec, tree arg)
       res.argmax = argmax;
     }
 
+  /* Clear KNOWNRANGE if the range has been adjusted to the maximum
+     of the directive.  If it has been cleared then since ARGMIN and/or
+     ARGMAX have been adjusted also adjust the corresponding ARGMIN and
+     ARGMAX in the result to include in diagnostics.  */
+  if (adjust_range_for_overflow (dirtype, &argmin, &argmax))
+    {
+      res.knownrange = false;
+      res.argmin = argmin;
+      res.argmax = argmax;
+    }
+
   /* Recursively compute the minimum and maximum from the known range,
      taking care to swap them if the lower bound results in longer
      output than the upper bound (e.g., in the range [-1, 0].  */
@@ -1776,13 +1825,13 @@ format_directive (const pass_sprintf_length::call_info &info,
      NUL that's appended after the format string has been processed.  */
   unsigned HOST_WIDE_INT navail = min_bytes_remaining (info.objsize, *res);
 
+  bool warned = res->warned;
+
   if (fmtres.range.min < fmtres.range.max)
     {
       /* The result is a range (i.e., it's inexact).  */
-      if (!res->warned)
+      if (!warned)
 	{
-	  bool warned = false;
-
 	  if (navail < fmtres.range.min)
 	    {
 	      /* The minimum directive output is longer than there is
@@ -1867,21 +1916,6 @@ format_directive (const pass_sprintf_length::call_info &info,
 				    navail);
 		}
 	    }
-
-	  res->warned |= warned;
-
-	  if (warned && fmtres.argmin)
-	    {
-	      if (fmtres.argmin == fmtres.argmax)
-		inform (info.fmtloc, "directive argument %qE", fmtres.argmin);
-	      else if (fmtres.bounded)
-		inform (info.fmtloc, "directive argument in the range [%E, %E]",
-			fmtres.argmin, fmtres.argmax);
-	      else
-		inform (info.fmtloc,
-			"using the range [%qE, %qE] for directive argument",
-			fmtres.argmin, fmtres.argmax);
-	    }
 	}
 
       /* Disable exact length checking but adjust the minimum and maximum.  */
@@ -1894,7 +1928,7 @@ format_directive (const pass_sprintf_length::call_info &info,
     }
   else
     {
-      if (!res->warned && fmtres.range.min > 0 && navail < fmtres.range.min)
+      if (!warned && fmtres.range.min > 0 && navail < fmtres.range.min)
 	{
 	  const char* fmtstr
 	    = (info.bounded
@@ -1909,10 +1943,10 @@ format_directive (const pass_sprintf_length::call_info &info,
 		  : G_("%<%.*s%> directive writing %wu byte "
 		       "into a region of size %wu")));
 
-	  res->warned = fmtwarn (dirloc, pargrange, NULL,
-				 OPT_Wformat_length_, fmtstr,
-				 (int)cvtlen, cvtbeg, fmtres.range.min,
-				 navail);
+	  warned = fmtwarn (dirloc, pargrange, NULL,
+			    OPT_Wformat_length_, fmtstr,
+			    (int)cvtlen, cvtbeg, fmtres.range.min,
+			    navail);
 	}
       *res += fmtres.range.min;
     }
@@ -1923,7 +1957,7 @@ format_directive (const pass_sprintf_length::call_info &info,
   if (!minunder4k || fmtres.range.max > 4095)
     res->under4k = false;
 
-  if (!res->warned && 1 < warn_format_length
+  if (!warned && 1 < warn_format_length
       && (!minunder4k || fmtres.range.max > 4095))
     {
       /* The directive output may be longer than the maximum required
@@ -1934,11 +1968,11 @@ format_directive (const pass_sprintf_length::call_info &info,
 	 (like Glibc does under some conditions).  */
 
       if (fmtres.range.min == fmtres.range.max)
-	res->warned = fmtwarn (dirloc, pargrange, NULL,
-			       OPT_Wformat_length_,
-			       "%<%.*s%> directive output of %wu bytes exceeds "
-			       "minimum required size of 4095",
-			       (int)cvtlen, cvtbeg, fmtres.range.min);
+	warned = fmtwarn (dirloc, pargrange, NULL,
+			  OPT_Wformat_length_,
+			  "%<%.*s%> directive output of %wu bytes exceeds "
+			  "minimum required size of 4095",
+			  (int)cvtlen, cvtbeg, fmtres.range.min);
       else
 	{
 	  const char *fmtstr
@@ -1948,17 +1982,17 @@ format_directive (const pass_sprintf_length::call_info &info,
 	       : G_("%<%.*s%> directive output between %qu and %wu "
 		    "bytes exceeds minimum required size of 4095"));
 
-	  res->warned = fmtwarn (dirloc, pargrange, NULL,
-				 OPT_Wformat_length_, fmtstr,
-				 (int)cvtlen, cvtbeg,
-				 fmtres.range.min, fmtres.range.max);
+	  warned = fmtwarn (dirloc, pargrange, NULL,
+			    OPT_Wformat_length_, fmtstr,
+			    (int)cvtlen, cvtbeg,
+			    fmtres.range.min, fmtres.range.max);
 	}
     }
 
   /* Has the minimum directive output length exceeded INT_MAX?  */
   bool exceedmin = res->number_chars_min > target_int_max ();
 
-  if (!res->warned
+  if (!warned
       && (exceedmin
 	  || (1 < warn_format_length
 	      && res->number_chars_max > target_int_max ())))
@@ -1967,11 +2001,11 @@ format_directive (const pass_sprintf_length::call_info &info,
 	 to exceed INT_MAX bytes.  */
 
       if (fmtres.range.min == fmtres.range.max)
-	res->warned = fmtwarn (dirloc, pargrange, NULL,
-			       OPT_Wformat_length_,
-			       "%<%.*s%> directive output of %wu bytes causes "
-			       "result to exceed %<INT_MAX%>",
-			       (int)cvtlen, cvtbeg, fmtres.range.min);
+	warned = fmtwarn (dirloc, pargrange, NULL,
+			  OPT_Wformat_length_,
+			  "%<%.*s%> directive output of %wu bytes causes "
+			  "result to exceed %<INT_MAX%>",
+			  (int)cvtlen, cvtbeg, fmtres.range.min);
       else
 	{
 	  const char *fmtstr
@@ -1980,12 +2014,27 @@ format_directive (const pass_sprintf_length::call_info &info,
 		     "bytes causes result to exceed %<INT_MAX%>")
 	       : G_ ("%<%.*s%> directive output between %wu and %wu "
 		     "bytes may cause result to exceed %<INT_MAX%>"));
-	  res->warned = fmtwarn (dirloc, pargrange, NULL,
-				 OPT_Wformat_length_, fmtstr,
-				 (int)cvtlen, cvtbeg,
-				 fmtres.range.min, fmtres.range.max);
+	  warned = fmtwarn (dirloc, pargrange, NULL,
+			    OPT_Wformat_length_, fmtstr,
+			    (int)cvtlen, cvtbeg,
+			    fmtres.range.min, fmtres.range.max);
 	}
     }
+
+  if (warned && fmtres.argmin)
+    {
+      if (fmtres.argmin == fmtres.argmax)
+	inform (info.fmtloc, "directive argument %qE", fmtres.argmin);
+      else if (fmtres.knownrange)
+	inform (info.fmtloc, "directive argument in the range [%E, %E]",
+		fmtres.argmin, fmtres.argmax);
+      else
+	inform (info.fmtloc,
+		"using the range [%E, %E] for directive argument",
+		fmtres.argmin, fmtres.argmax);
+    }
+
+  res->warned |= warned;
 }
 
 /* Account for the number of bytes between BEG and END (or between
@@ -2057,7 +2106,7 @@ add_bytes (const pass_sprintf_length::call_info &info,
 	 indicate that the overflow/truncation may (but need not) happen.  */
       bool boundrange
 	= (res->number_chars_min < res->number_chars_max
-	   && res->number_chars_min < info.objsize);
+	   && res->number_chars_min + nbytes <= info.objsize);
 
       if (!end && ((nbytes - navail) == 1 || boundrange))
 	{
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78622.c b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
new file mode 100644
index 0000000..44c9b0b
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
@@ -0,0 +1,39 @@
+/* PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value
+   incorrect with overflow/wrapping */
+/* { dg-additional-options "-Wformat-length=2" } */
+
+__attribute__((noinline, noclone)) int
+foo (int x)
+{
+  if (x < 4096 + 8 || x >= 4096 + 256 + 8)
+    return -1;
+  char buf[5];
+  return __builtin_sprintf (buf, "%hhd", x + 1);
+}
+
+int
+bar (unsigned int x)
+{
+  if (x < 64 || x > 2U * __INT_MAX__ - 10)
+    return -1;
+  char buf[1];
+  return __builtin_sprintf (buf, "%d", x + 1);   /* { dg-warn "directive writing between 1 and 11 bytes into a region of size 1" "int32plus" { target { int32plus } } */
+}
+
+int
+main ()
+{
+  if (__SCHAR_MAX__ != 127 || __CHAR_BIT__ != 8 || __SIZEOF_INT__ != 4)
+    return 0;
+  if (foo (4095 + 9) != 1
+      || foo (4095 + 32) != 2
+      || foo (4095 + 127) != 3
+      || foo (4095 + 128) != 4
+      || foo (4095 + 240) != 3
+      || foo (4095 + 248) != 2
+      || foo (4095 + 255) != 2
+      || foo (4095 + 256) != 1)
+    __builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
index 4dddccdf..260f4fc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
@@ -25,6 +25,9 @@ char buf8k [8192];
 #define concat(a, b)   a ## b
 #define CAT(a, b)      concat (a, b)
 
+/* Calls to this function must not be eliminated.  */
+void must_not_eliminate (void);
+
 #define EQL(expect, size, fmt, ...)					\
   void __attribute__ ((noinline, noclone))				\
   CAT (test_on_line_, __LINE__)(void)					\
@@ -34,7 +37,7 @@ char buf8k [8192];
 	char *dst = size < 0 ? buf : buf8k + sizeof buf8k - size;	\
 	int result = __builtin_sprintf (dst, fmt, __VA_ARGS__);		\
 	if (result != expect)						\
-	  __builtin_abort ();						\
+	  must_not_eliminate ();					\
       }									\
   }
 
@@ -50,7 +53,7 @@ char buf8k [8192];
 	char *dst = size < 0 ? buf : buf8k + sizeof buf8k - size;	\
 	int result = __builtin_sprintf (dst, fmt, __VA_ARGS__);		\
 	if (result < min || max < result)				\
-	  __builtin_abort ();						\
+	  must_not_eliminate ();					\
       }									\
   }
 
@@ -75,6 +78,8 @@ EQL (0, 0, "%-s", "");
 EQL (1, 1, "%c",  'x');
 EQL (1, 1, "%-s", "x");
 
+EQL (1, 2, "%c",  'x');
+
 EQL (4, 4, "%4c", 'x');
 
 /* Verify that exceeding the environmental limit of 4095 bytes for
@@ -179,7 +184,7 @@ RNG (4,  4, 32, "%zu", sz)
 
 /* Exercise bug 78586.  */
 RNG (4,  4, 32, "%lu", (unsigned long)i)
-RNG (4,  4, 32, "%lu", (unsigned)u)
+RNG (4,  4, 32, "%lu", (unsigned long)u)
 RNG (4,  4, 32, "%lu", (unsigned long)li)
 RNG (4,  4, 32, "%lu", (unsigned long)lu)
 RNG (4,  4, 32, "%lu", (unsigned long)sz)
@@ -259,21 +264,24 @@ RNG (0,  6,   8, "%s%ls", "1", L"2");
 /* Verify that no call to abort has been eliminated and that each call
    is at the beginning of a basic block (and thus the result of a branch).
    This latter test tries to verify that the test preceding the call to
-   abort has not been eliminated either.
+   the must_not_eliminate() function has not been eliminated either.
 
    The expected output looks something like this:
 
    <bb 2>:
    result_3 = __builtin_sprintf (&MEM[(void *)&buf8k + 8192B], "%c", 32);
    if (result_3 != 0)
-     goto <bb 3>;
+     goto <bb 3>; [50.0%]
    else
-     goto <bb 4>;
+     goto <bb 4>; [50.0%]
 
-   <bb 3>:
-   __builtin_abort ();
+   <bb 3>[50.0%]:
+   must_not_eliminate ();
 
 */
 
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 124 "optimized" { target { ilp32 || lp64 } } } } */
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } } */
+/*  Only conditional calls to abort should be made (with any probability):
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *must_not_eliminate" 124 "optimized" { target { ilp32 || lp64 } } } }
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *must_not_eliminate" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } }
+    No unconditional calls to abort should be made:
+    { dg-final { scan-tree-dump-not ";\n *must_not_eliminate" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
index dbb0dd9..c4489ac 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
@@ -8,18 +8,20 @@
 #define FAIL(line)  CAT (failure_on_line_, line)
 #define PASS(line)  CAT (success_on_line_, line)
 
-/* Emit a call to a function named failure_on_line_NNN when EXPR is false.  */
-#define ASSERT(value, expect)			\
+/* Emit a call to a function named failure_on_line_NNN when VALUE
+   is not equal to the constant EXPECTED, otherwise emit a call to
+   function success_on_line_NNN.  */
+#define ASSERT(value, expected)			\
   do {						\
     extern void FAIL (__LINE__)(int);		\
     extern void PASS (__LINE__)(int);		\
-    if (value == expect)			\
+    if (value == expected)			\
       PASS (__LINE__)(value);			\
     else					\
       FAIL (__LINE__)(value);			\
   } while (0)
 
-#define T(expect, ...)					\
+#define EQL(expect, ...)				\
   do {							\
     int n = __builtin_snprintf (0, 0, __VA_ARGS__);	\
     ASSERT (n, expect);					\
@@ -27,88 +29,118 @@
 
 int ival (int i) { return i; }
 
+/* Generate a signed int value in the specified range.  */
+
+static int
+int_range (int min, int max)
+{
+  extern int int_value (void);
+  int val = int_value ();
+  if (val < min || max < val)
+    val = min;
+  return val;
+}
+
+#define R(min, max) int_range (min, max)
+
 void test_arg_int (int i, int n)
 {
-  T (1, "%i", ival (0));
-  T (1, "%i", ival (1));
-  T (2, "%i%i", ival (0), ival (1));
-  T (3, "%i%i%i", ival (0), ival (1), ival (9));
-  T (5, "%i %i %i", ival (0), ival (1), ival (9));
+  EQL (1, "%i", ival (0));
+  EQL (1, "%i", ival (1));
+  EQL (2, "%i%i", ival (0), ival (1));
+  EQL (3, "%i%i%i", ival (0), ival (1), ival (9));
+  EQL (5, "%i %i %i", ival (0), ival (1), ival (9));
 
-  T (5, "%i %i %i", ival (0), ival (1), ival (9));
+  EQL (5, "%i %i %i", ival (0), ival (1), ival (9));
 
-  T (13, "%hhu.%hhu.%hhu.%hhu", ival (23), ival (78), ival (216), ival (135));
+  EQL (13, "%hhu.%hhu.%hhu.%hhu", ival (23), ival (78), ival (216), ival (135));
 
   for (i = 0; i != 9; ++i)
-    T (1, "%i", i);
+    EQL (1, "%i", i);
 
   for (i = -n; i != n; ++i)
-    T (8, "%08x", i);
+    EQL (8, "%08x", i);
 
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
-  T (0, "%.0d", ival (0));
-  T (0, "%.0i", ival (0));
-  T (0, "%.0o", ival (0));
-  T (0, "%.0u", ival (0));
-  T (0, "%.0x", ival (0));
-
-  T (0, "%.*d", 0, ival (0));
-  T (0, "%.*i", 0, ival (0));
-  T (0, "%.*o", 0, ival (0));
-  T (0, "%.*u", 0, ival (0));
-  T (0, "%.*x", 0, ival (0));
-
-  T (1, "%1.0d", ival (0));
-  T (1, "%1.0i", ival (0));
-  T (1, "%1.0o", ival (0));
-  T (1, "%1.0u", ival (0));
-  T (1, "%1.0x", ival (0));
+  EQL (0, "%.0d", ival (0));
+  EQL (0, "%.0i", ival (0));
+  EQL (0, "%.0o", ival (0));
+  EQL (0, "%.0u", ival (0));
+  EQL (0, "%.0x", ival (0));
+
+  EQL (0, "%.*d", 0, ival (0));
+  EQL (0, "%.*i", 0, ival (0));
+  EQL (0, "%.*o", 0, ival (0));
+  EQL (0, "%.*u", 0, ival (0));
+  EQL (0, "%.*x", 0, ival (0));
+
+  EQL (1, "%1.0d", ival (0));
+  EQL (1, "%1.0i", ival (0));
+  EQL (1, "%1.0o", ival (0));
+  EQL (1, "%1.0u", ival (0));
+  EQL (1, "%1.0x", ival (0));
+
+  EQL (4, "%hhi", R (-128, -127));
+  EQL (3, "%hhi", R ( -99,  -10));
+  EQL (2, "%hhi", R (  -9,   -1));
+  EQL (1, "%hhi", R (   0,    9));
+  EQL (1, "%hhi", R (   0,    9));
+
+  EQL (1, "%1.0hhi", R (   0,    1));
+  EQL (1, "%1.1hhi", R (   0,    9));
+  EQL (2, "%1.2hhi", R (   0,    9));
+  EQL (3, "%1.3hhi", R (   0,    9));
+
+  EQL (1, "%hhi", R (1024, 1033));
+  EQL (2, "%hhi", R (1034, 1123));
+  EQL (1, "%hhu", R (1024, 1033));
+  EQL (2, "%hhu", R (1034, 1123));
 }
 
 void test_arg_string (const char *s)
 {
-  T ( 0, "%-s", "");
-  T ( 1, "%%");
-  T ( 1, "%-s", "1");
-  T ( 2, "%-s", "12");
-  T ( 3, "%-s", "123");
-  T ( 5, "s=%s", "123");
-  T (10, "%%s=\"%s\"", "12345");
-
-  T ( 1, "%.*s", 1, "123");
-  T ( 2, "%.*s", 2, "123");
-  T ( 3, "%.*s", 3, "123");
-  T ( 3, "%.*s", 4, "123");
-
-  T ( 1, "%1.*s", 1, "123");
-  T ( 2, "%1.*s", 2, "123");
-  T ( 3, "%1.*s", 3, "123");
-  T ( 3, "%1.*s", 4, "123");
-  T ( 4, "%4.*s", 1, "123");
-  T ( 4, "%4.*s", 2, "123");
-  T ( 4, "%4.*s", 3, "123");
-  T ( 4, "%4.*s", 4, "123");
-  T ( 4, "%4.*s", 5, "123");
+  EQL ( 0, "%-s", "");
+  EQL ( 1, "%%");
+  EQL ( 1, "%-s", "1");
+  EQL ( 2, "%-s", "12");
+  EQL ( 3, "%-s", "123");
+  EQL ( 5, "s=%s", "123");
+  EQL (10, "%%s=\"%s\"", "12345");
+
+  EQL ( 1, "%.*s", 1, "123");
+  EQL ( 2, "%.*s", 2, "123");
+  EQL ( 3, "%.*s", 3, "123");
+  EQL ( 3, "%.*s", 4, "123");
+
+  EQL ( 1, "%1.*s", 1, "123");
+  EQL ( 2, "%1.*s", 2, "123");
+  EQL ( 3, "%1.*s", 3, "123");
+  EQL ( 3, "%1.*s", 4, "123");
+  EQL ( 4, "%4.*s", 1, "123");
+  EQL ( 4, "%4.*s", 2, "123");
+  EQL ( 4, "%4.*s", 3, "123");
+  EQL ( 4, "%4.*s", 4, "123");
+  EQL ( 4, "%4.*s", 5, "123");
 
   const char *a = "123";
   const char *b = "456";
 
-  T ( 3, "%-s", s ? a : b);
-  T ( 0, "%.0s", s);
-  T ( 1, "%1.1s", s);
-  T ( 2, "%2.2s", s);
-  T ( 2, "%2.1s", s);
+  EQL ( 3, "%-s", s ? a : b);
+  EQL ( 0, "%.0s", s);
+  EQL ( 1, "%1.1s", s);
+  EQL ( 2, "%2.2s", s);
+  EQL ( 2, "%2.1s", s);
 }
 
 void test_arg_multiarg (int i, double d)
 {
-  T (16, "%i %f %s", 123, 3.14, "abc");
-  T (16, "%12i %s", i, "abc");
-  T (16, "%*i %s", 12, i, "abc");
+  EQL (16, "%i %f %s", 123, 3.14, "abc");
+  EQL (16, "%12i %s", i, "abc");
+  EQL (16, "%*i %s", 12, i, "abc");
 }
 
-#define TV(expect, fmt, va)				\
+#define EQLv(expect, fmt, va)				\
   do {							\
     int n = __builtin_vsnprintf (0, 0, fmt, va);	\
     ASSERT (n, expect);					\
@@ -116,21 +148,21 @@ void test_arg_multiarg (int i, double d)
 
 void test_va_int (__builtin_va_list va)
 {
-  TV ( 2, "%02hhx", va);
-  TV ( 2, "%02.*hhx", va);
-  TV ( 4, "%04hx", va);
-  TV ( 4, "%04.*hx", va);
+  EQLv ( 2, "%02hhx", va);
+  EQLv ( 2, "%02.*hhx", va);
+  EQLv ( 4, "%04hx", va);
+  EQLv ( 4, "%04.*hx", va);
 }
 
 void test_va_multiarg (__builtin_va_list va)
 {
-  TV ( 8, "%8x", va);
-  TV ( 8, "% 8x", va);
-  TV ( 9, "%9x", va);
-  TV (11, "%11o", va);
-  TV (12, "%12o", va);
+  EQLv ( 8, "%8x", va);
+  EQLv ( 8, "% 8x", va);
+  EQLv ( 9, "%9x", va);
+  EQLv (11, "%11o", va);
+  EQLv (12, "%12o", va);
 
-  TV (16, "%12i %3.2s", va);
+  EQLv (16, "%12i %3.2s", va);
 }
 
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
index 4c41234..abd49df 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
@@ -8,6 +8,8 @@
    { dg-options "-O2 -Wformat -fdump-tree-optimized" }
    { dg-require-effective-target int32plus } */
 
+typedef __SIZE_TYPE__ size_t;
+
 #define CONCAT(a, b) a ## b
 #define CAT(a, b)    CONCAT (a, b)
 
@@ -50,6 +52,19 @@ void test_arg_int (int width, int prec, int i, int n)
 
   T ("%i", R (1, 10));
 
+  /* Each of the bounds of the ranges below results in just one byte
+     on output because they convert to the value 9 in type char, yet
+     other values in those ranges can result in up to four bytes.
+     For example, 4240 converts to -112.  Verify that the output
+     isn't folded into a constant.  This assumes __CHAR_BIT__ of 8.  */
+  T ("%hhi", R (4104, 4360) + 1);
+  T ("%hhu", R (4104, 4360) + 1);
+
+  /* Here, the range includes all possible lengths of output for
+     a 16-bit short and 32-bit int.  */
+  T ("%hi", R (65536, 65536 * 2));
+  T ("%hu", R (65536, 65536 * 2));
+
   T ("%'i", 1234567);
 
   for (i = -n; i != n; ++i)
@@ -104,6 +119,7 @@ void test_invalid_directive (void)
   T ("abc%Q");    /* { dg-warning "unknown conversion type character .Q." } */
 }
 
+
 /* Use 'grep "^ *T (" builtin-sprintf-6.c  | wc -l' to determine
    the count for the directive below.
-   { dg-final { scan-tree-dump-times "snprintf" 42 "optimized"} } */
+   { dg-final { scan-tree-dump-times "snprintf" 46 "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
index fae584e..f49422a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -1039,8 +1039,8 @@ void test_sprintf_chk_s_nonconst (int w, int p, const char *s)
      is not.  */
   T ( 1, "%*s",  w, "");
   T ( 1, "%*s",  w, "1");       /* { dg-warning "nul past the end" } */
-  T ( 1, "%.*s", w, "");
-  T ( 1, "%.*s", w, "1");       /* { dg-warning "may write a terminating nul" } */
+  T ( 1, "%.*s", p, "");
+  T ( 1, "%.*s", p, "1");       /* { dg-warning "may write a terminating nul" } */
   T ( 1, "%.*s", w, "123");     /* { dg-warning "writing between 0 and 3 bytes into a region of size 1" } */
 
   T ( 1, "%*s", w, "123");      /* { dg-warning "writing 3 or more bytes into a region of size 1" } */
@@ -1294,6 +1294,12 @@ void test_sprintf_chk_int_nonconst (int w, int p, int a)
   T (3, "%2x",          a);
 
   T (1, "%.*d",      p, a);
+
+  T (4, "%i %i",        a, a);
+  /* The following will definitely be "writing a terminating nul past the end"
+     (i.e., not "may write".)  */
+  T (4, "%i %i ",       a, a);      /* { dg-warning "writing a terminating nul past the end" } */
+  T (4, "%i %i %i",     a, a, a);   /* { dg-warning "into a region" }*/
 }
 
 void test_sprintf_chk_e_nonconst (int w, int p, double d)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
index 00176ed..21d27bf 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
@@ -116,54 +116,76 @@ void test_sprintf_chk_integer_value (void)
   T ( 9, "%8u", i (    1));
 }
 
+extern int rand (void);
+
 /* Functions to require optimization to figure out the range of the operand.
    Used to verify that the checker makes use of the range information to
    avoid diagnosing the output of sufficiently constrained arguments to
    integer directives.  */
 
-signed char*
-range_schar (signed char *val, signed char min, signed char max)
+signed char
+range_schar (signed char min, signed char max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  signed char val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-unsigned char*
-range_uchar (unsigned char *val, unsigned char min, unsigned char max)
+unsigned char
+range_uchar (unsigned char min, unsigned char max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  unsigned char val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-signed short*
-range_sshort (signed short *val, signed short min, signed short max)
+signed short
+range_sshrt (signed short min, signed short max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  signed short val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-unsigned short*
-range_ushort (unsigned short *val, unsigned short min, unsigned short max)
+unsigned short
+range_ushrt (unsigned short min, unsigned short max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  unsigned short val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-/* Helper to prevent GCC from figuring out the return value.  */
-extern int idx (void);
+signed int
+range_sint (signed int min, signed int max)
+{
+  signed int val = rand ();
+  return val < min || max < val ? min : val;
+}
 
-/* Exercise ranges only in types signed and unsigned char and short.
-   No other types work due to bug 71690.  */
+unsigned int
+range_uint (unsigned int min, unsigned int max)
+{
+  unsigned int val = rand ();
+  return val < min || max < val ? min : val;
+}
 
-void test_sprintf_chk_range_schar (signed char *a)
+void test_sprintf_chk_range_schar (void)
 {
-  (void)&a;
+#define R(min, max) range_sint (min, max)
+
+  T ( 0, "%hhi", R (0, 1));     /* { dg-warning ".%hhi. directive writing 1 byte into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[0, 1\\\]" "note" { target *-*-* } .-1 } */
 
-  /* Ra creates a range of signed char for A [idx].  A different
-     value is used each time to prevent the ranges from intesecting
-     one another, possibly even eliminating some tests as a result
-     of the range being empty.  */
-#define R(min, max) *range_schar (a + idx (), min, max)
+  T ( 0, "%hhi", R (0, 127));   /* { dg-warning ".%hhi. directive writing between 1 and 3 bytes into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[0, 127\\\]" "note" { target *-*-* } .-1 } */
+
+  T ( 0, "%hhi", R (1024, 1033));   /* { dg-warning ".%hhi. directive writing 1 byte into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[1024, 1033\\\]" "note" { target *-*-* } .-1 } */
+
+  T ( 0, "%hhi", R (1024, 1034));   /* { dg-warning ".%hhi. directive writing between 1 and 2 bytes into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[1024, 1034\\\]" "note" { target *-*-* } .-1 } */
+
+  T ( 0, "%hhi", R (1024, 2035));   /* { dg-warning ".%hhi. directive writing between 1 and 4 bytes into a region of size 0" } */
+  /* { dg-message "using the range \\\[0, -128\\\] for directive argument" "note" { target *-*-* } .-1 } */
+
+#undef R
+#define R(min, max) range_schar (min, max)
 
   T ( 0, "%i",  R (0, 9));      /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
   T ( 1, "%i",  R (0, 9));      /* { dg-warning "nul past the end" } */
@@ -192,47 +214,108 @@ void test_sprintf_chk_range_schar (signed char *a)
   T ( 6, "%i_%i_%i", R (0, 9), R (0, 10), R (0, 10)); /* { dg-warning "may write a terminating nul past the end|.%i. directive writing between 1 and 2 bytes into a region of size 1" } */
 }
 
-void test_sprintf_chk_range_uchar (unsigned char *a, unsigned char *b)
+void test_sprintf_chk_range_uchar (void)
+{
+#undef R
+#define R(min, max) range_uchar (min, max)
+
+  T ( 0, "%i",  R (0,  9));   /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R (0,  9));   /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R (0,  9));
+  T ( 2, "%i",  R (9, 10));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R (0,  99));
+  T ( 3, "%i",  R (0, 100));  /* { dg-warning "may write a terminating nul past the end of the destination" } */
+}
+
+void test_sprintf_chk_range_sshrt (void)
+{
+#undef R
+#define R(min, max) range_sshrt (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R (-1, 0));     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
+}
+
+void test_sprintf_chk_range_ushrt (void)
 {
-  (void)&a;
-  (void)&b;
+#undef R
+#define R(min, max) range_ushrt (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
+}
 
-#undef Ra
-#define Ra(min, max) *range_uchar (a + idx (), min, max)
+void test_sprintf_chk_range_sint (void)
+{
+#undef R
+#define R(min, max) range_sint (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R (-1, 0));     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
 
-  T ( 0, "%i",  Ra (0,  9));   /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
-  T ( 1, "%i",  Ra (0,  9));   /* { dg-warning "nul past the end" } */
-  T ( 2, "%i",  Ra (0,  9));
-  T ( 2, "%i",  Ra (9, 10));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
 
-  T ( 3, "%i",  Ra (0,  99));
-  T ( 3, "%i",  Ra (0, 100));  /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
 }
 
-void test_sprintf_chk_range_sshort (signed short *a, signed short *b)
+void test_sprintf_chk_range_uint (void)
 {
-  (void)&a;
-  (void)&b;
-
-#undef Ra
-#define Ra(min, max) *range_sshort (a + idx (), min, max)
-
-  T ( 0, "%i",  Ra ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
-  T ( 1, "%i",  Ra ( 0, 1));     /* { dg-warning "nul past the end" } */
-  T ( 1, "%i",  Ra ( 0, 9));     /* { dg-warning "nul past the end" } */
-  T ( 2, "%i",  Ra ( 0, 1));
-  T ( 2, "%i",  Ra ( 8, 9));
-  T ( 2, "%i",  Ra ( 0, 9));
-  T ( 2, "%i",  Ra (-1, 0));     /* { dg-warning "may write a terminating nul past the end of the destination" } */
-  T ( 2, "%i",  Ra ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
-
-  T ( 3, "%i",  Ra ( 0, 99));
-  T ( 3, "%i",  Ra (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
-
-  T ( 4, "%i",  Ra (  0,  999));
-  T ( 4, "%i",  Ra ( 99,  999));
-  T ( 4, "%i",  Ra (998,  999));
-  T ( 4, "%i",  Ra (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
+#undef R
+#define R(min, max) range_uint (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
 }
 
 /* Verify that destination size in excess of INT_MAX (and, separately,
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
index 0cb02b7..121ed4e 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
@@ -2,15 +2,18 @@
    Test to verify that the correct range information is made available to the
    -Wformat-lenght check to prevent warnings.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wformat -Wformat-length" } */
+/* { dg-options "-O2 -Wformat -Wformat-length -fdump-tree-optimized" } */
 
+void abort (void);
 int snprintf (char*, __SIZE_TYPE__, const char*, ...);
 
 void fuchar (unsigned char j, char *p)
 {
   if (j > 99)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fschar (signed char j, char *p)
@@ -20,14 +23,17 @@ void fschar (signed char j, char *p)
   if (k > 99)
     return;
 
-  snprintf (p, 3, "%3hhu", k);   /* { dg-bogus "" "unsigned char" { xfail *-*-* } } */
+  if (3 != snprintf (p, 4, "%3hhu", k))
+    abort ();
 }
 
 void fushrt (unsigned short j, char *p)
 {
   if (j > 999)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fshrt (short j, char *p)
@@ -37,7 +43,8 @@ void fshrt (short j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3hu", k);
+  if (3 != snprintf (p, 4, "%3hu", k))
+    abort ();
 }
 
 void fuint (unsigned j, char *p)
@@ -54,13 +61,16 @@ void fint (int j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3u", k);   /* { dg-bogus "" "unsigned int" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3u", k);
 }
 
 void fulong (unsigned long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3lu", j);
 }
 
@@ -71,13 +81,16 @@ void flong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3lu", k);   /* { dg-bogus "" "unsigned long" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3lu", k);
 }
 
 void fullong (unsigned long long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3llu", j);
 }
 
@@ -88,5 +101,7 @@ void fllong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3llu", k);   /* { dg-bogus "" "unsigned long long" { xfail lp64 } } */
+  snprintf (p, 4, "%3llu", k);
 }
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
index 916df79..35a5bd0 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
@@ -166,27 +166,48 @@ test_c (char c)
   EQL (5,  6, "%c %c %c",  c,   c,   c);
 }
 
-/* Generate a pseudo-random value in the specified range.  The return
-   value must be unsigned char to work around limitations in the GCC
-   range information.  Similarly for the declaration of rand() whose
-   correct return value should be int, but that also prevents the range
-   information from making it to the printf pass.  */
+/* Generate a pseudo-random unsigned value.  */
 
-unsigned char uchar_range (unsigned min, unsigned max)
+unsigned __attribute__ ((noclone, noinline))
+unsigned_value (void)
 {
-  extern unsigned rand (void);
+  extern int rand ();
+  return rand ();
+}
 
-  unsigned x;
-  x = rand ();
+/* Generate a pseudo-random signed value.  */
 
-  if (x < min)
-    x = min;
-  else if (max < x)
-    x = max;
+int __attribute__ ((noclone, noinline))
+int_value (void)
+{
+  extern int rand ();
+  return rand ();
+}
+
+/* Generate an unsigned char value in the specified range.  */
 
+static unsigned char
+uchar_range (unsigned min, unsigned max)
+{
+  unsigned x = unsigned_value ();
+  if (x < min || max < x)
+    x = min;
   return x;
 }
 
+/* Generate a signed int value in the specified range.  */
+
+static int
+int_range (int min, int max)
+{
+  int val = int_value ();
+  if (val < min || max < val)
+    val = min;
+  return val;
+}
+
+#define IR(min, max) int_range (min, max)
+
 static void __attribute__ ((noinline, noclone))
 test_d_i (int i, long li)
 {
@@ -266,9 +287,35 @@ test_d_i (int i, long li)
   RNG ( 1,  4,  5, "%hhi",     i);
   RNG ( 1,  3,  4, "%hhu",     i);
 
+  RNG ( 3,  4,  5, "%hhi",     IR (-128,  -10));
+  RNG ( 2,  4,  5, "%hhi",     IR (-128,   -1));
+  RNG ( 1,  4,  5, "%hhi",     IR (-128,    0));
+
+  RNG ( 1,  4,  5, "%1hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%2hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%3hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%4hhi",    IR (-128,    0));
+  RNG ( 1,  5,  6, "%5hhi",    IR (-128,    0));
+  RNG ( 1,  6,  7, "%6hhi",    IR (-128,    0));
+  RNG ( 2,  6,  7, "%6hhi",    IR (-128,   10));
+
+  RNG ( 0,  1,  2, "%.hhi",    IR (   0,    1));
+  RNG ( 0,  1,  2, "%.0hhi",   IR (   0,    1));
+  RNG ( 0,  1,  2, "%0.0hhi",  IR (   0,    1));   /* { dg-warning ".0. flag ignored with precision" } */
+  RNG ( 0,  1,  2, "%*.0hhi",  0, IR (   0,    1));
+
+  RNG ( 1,  2,  3, "%hhi",     IR (1024, 1034));
+  RNG ( 1,  4,  5, "%hhi",     IR (1024, 2048));
+  RNG ( 2,  3,  4, "%hhi",     IR (1034, 1151));
+
+  RNG ( 1,  2,  3, "%hhu",     IR (1024, 1034));
+  RNG ( 1,  3,  4, "%hhu",     IR (1024, 2048));
+  RNG ( 2,  3,  4, "%hhu",     IR (1034, 1151));
+
 #if __SIZEOF_SHORT__ == 2
   RNG ( 1,  6,  7, "%hi",      i);
   RNG ( 1,  5,  6, "%hu",      i);
+
 #elif __SIZEOF_SHORT__ == 4
   RNG ( 1, 11, 12, "%hi",      i);
   RNG ( 1, 10, 11, "%hu",      i);

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jeff Law]

On 12/02/2016 03:54 PM, Martin Sebor wrote:
> Thanks for looking at this!  I realize it's dense code and not easy
> to make sense out of.
>>> +
>>> +  if (TREE_CODE (*argmin) == INTEGER_CST
>>> +      && TREE_CODE (*argmax) == INTEGER_CST
>>> +      && (dirprec >= argprec
>>> +      || integer_zerop (int_const_binop (RSHIFT_EXPR,
>>> +                         int_const_binop (MINUS_EXPR,
>>> +                                  *argmax,
>>> +                                  *argmin),
>>> +                         size_int (dirprec)))))
>>> +  {
>>> +    *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0,
>>> false);
>>> +    *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0,
>>> false);
>>> +    return false;
>>> +  }
>> So in this case we're not changing the values, we're just converting
>> them to a equal or wider type, right?  Thus no adjustment (what about a
>> sign change?  Is that an adjustment?) and we return false per the
>> function's specifications.
>
> This casts the value to the destination type, so it may result in
> different values.  The important postcondition it preserves is that
> the difference ARGMAX - ARGMIN is less than or equal to TYPE_MAX of
> the directive's unsigned TYPE.  (If it isn't, the range cannot be
> converted.)  This lets us take, say:
>
>   int f (int i)
>   {
>     if (i < 1024 || 1033 < i)
>        i = 1024;
>
>     return snprintf (0, 0, "%hhi", i);
>   }
>
> and fold the function call into 1 because (signed char)1024 is 0 and
> (signed char)1033 is 9, and every other value in that same original
> range is also in the same range after the conversion.  We know it's
> safe because (1033 - 1024 <= UCHAR_MAX) holds.
But the code in question is guarded by dirprec >= argprec.  Thus aren't 
we converting to an equal or wider type?  In the case of converting to a 
wider type, ISTM the values won't change and thus we return false.

If we are converting to the same sized type, but with a different 
signedness, then the values will have been adjusted and ISTM we ought to 
be returning true in that case.  But the code actually returns false.

I must be missing something here.





>
>> What about overflows when either argmin or argmax won't fit into dirtype
>> without an overflow?  What's the right behavior there?
>
> That's fine just as long as the property above holds.
>
> I think the algorithm works but the code came from tree-vrp where
> there are all sorts of additional checks for some infinities which
> VRP distinguishes from type limits like SCHAR_MIN and _MAX.  I don't
> fully understand those and so I'd feel better if someone who does
> double checked this code.
That's what prompted my question about overflows.  It was a general 
concern after reading the VRP code.  I did not have a specific case in 
mind that would be mis-handled.


>
>>
>>> +
>>> +  tree dirmin = TYPE_MIN_VALUE (dirtype);
>>> +  tree dirmax = TYPE_MAX_VALUE (dirtype);
>> From this point onward argmin/argmax are either not integers or we're
>> doing a narrowing conversion, right?  In both cases the fact that we're
>> doing a narrowing conversion constrains the range
>
> Argmin and argmax are always integers.  The rest of the function
> handles the case where the postcondition above doesn't hold (e.g.,
> in function g above).
OK.  So is the hangup really a problem in how the return type is 
documented?  I parsed the comment as essentially saying we return true 
if the range gets adjusted in any way -- thus a sign change in the first 
block would qualify, but we returned false which seemed inconsistent.

Looking at it again, what I think it's saying is we're returning true 
only for a subset of adjustments to the range, ie, those cases where the 
postcondition does not hold.  Correct?


>
>>
>>> +
>>> +  if (TYPE_UNSIGNED (dirtype))
>>> +    {
>>> +      *argmin = dirmin;
>>> +      *argmax = dirmax;
>>> +    }
>>> +  else
>>> +    {
>>> +      *argmin = integer_zero_node;
>>> +      *argmax = dirmin;
>>> +    }
>> Should this be dirmax? Am I missing something here?
>
> It's dirmin because for a signed type, due to the sign, it results
> in more bytes on output than dirmin would.  (E.g., with SCHAR_MIN
> = -128 and SCHAR_MAX = 127 it's four bytes vs three.)
Ah.  I understand that.  THanks for clarifying.

jeff

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jeff Law]

On 12/05/2016 08:43 PM, Martin Sebor wrote:

> Martin
>
> $ cat b.c && /build/gcc-78622/gcc/xgcc -B /build/gcc-78622/gcc -O2 -S
> -Wall -Wextra -Wpedantic b.c
> char d[1];
>
> void f (int i)
> {
>   if (i < 1024 || 1033 < i) i = 1024;
>
>   __builtin_sprintf (d + 1, "%hhi", i);
> }
>
> void g (int i)
> {
>   if (i < 1024 || 3456 < i) i = 1024;
>
>   __builtin_sprintf (d + 1, "%hhi", i);
> }
>
> b.c: In function ‘f’:
> b.c:7:30: warning: ‘%hhi’ directive writing 1 byte into a region of size
> 0 [-Wformat-length=]
>    __builtin_sprintf (d + 1, "%hhi", i);
>                               ^~~~
> b.c:7:29: note: directive argument in the range [1024, 1033]
>    __builtin_sprintf (d + 1, "%hhi", i);
>                              ^~~~~~
> b.c:7:3: note: format output 2 bytes into a destination of size 0
>    __builtin_sprintf (d + 1, "%hhi", i);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> b.c: In function ‘g’:
> b.c:14:30: warning: ‘%hhi’ directive writing between 1 and 4 bytes into
> a region of size 0 [-Wformat-length=]
>    __builtin_sprintf (d + 1, "%hhi", i);
>                               ^~~~
> b.c:14:29: note: using the range [0, -128] for directive argument
>    __builtin_sprintf (d + 1, "%hhi", i);
>                              ^~~~~~
> b.c:14:3: note: format output between 2 and 5 bytes into a destination
> of size 0
>    __builtin_sprintf (d + 1, "%hhi", i);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> gcc-78622.diff
>
>
> PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping
>
> gcc/ChangeLog:
>
> 	PR middle-end/78622
> 	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
> 	rather than res.bounded.
> 	(adjust_range_for_overflow): New function.
> 	(format_integer): Always set res.bounded to true unless either
> 	precision or width is specified and unknown.
> 	Call adjust_range_for_overflow.
> 	(format_directive): Remove vestigial quoting.  Always inform of
> 	argument value or range when it's available.
> 	(add_bytes): Correct the computation of boundrange used to
> 	decide whether a warning is of a "maybe" or "defnitely" kind.
>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/78622
> 	* gcc.c-torture/execute/pr78622.c: New test.
> 	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
> 	behavior inadvertently introduced in a previous commit.  Tighten
> 	up final checking.
> 	* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
> 	Add test cases.
> 	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
> 	add a final optimization check.
> 	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.
>
> diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
> index 99a635a..a888f55 100644
> --- a/gcc/gimple-ssa-sprintf.c
> +++ b/gcc/gimple-ssa-sprintf.c
> @@ -795,6 +795,59 @@ get_width_and_precision (const conversion_spec &spec,
>    *pprec = prec;
>  }
>
> +/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
> +   argument, due to the conversion from either *ARGMIN or *ARGMAX to
> +   the type of the directive's formal argument it's possible for both
> +   to result in the same number of bytes or a range of bytes that's
> +   less than the number of bytes that would result from formatting
> +   some other value in the range [*ARGMIN, *ARGMAX].  This can be
> +   determined by checking for the actual argument being in the range
> +   of the type of the directive.  If it isn't it must be assumed to
> +   take on the full range of the directive's type.
> +   Return true when the range has been adjusted to the range of
> +   DIRTYPE, false otherwise.  */
So I think the return value needs a bit of clarification here.  Take 
guidance from our discussion on this thread.

OK with that fixed.

jeff

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Martin Sebor]

On 12/07/2016 11:43 AM, Jeff Law wrote:
> On 12/02/2016 03:54 PM, Martin Sebor wrote:
>> Thanks for looking at this!  I realize it's dense code and not easy
>> to make sense out of.
>>>> +
>>>> +  if (TREE_CODE (*argmin) == INTEGER_CST
>>>> +      && TREE_CODE (*argmax) == INTEGER_CST
>>>> +      && (dirprec >= argprec
>>>> +      || integer_zerop (int_const_binop (RSHIFT_EXPR,
>>>> +                         int_const_binop (MINUS_EXPR,
>>>> +                                  *argmax,
>>>> +                                  *argmin),
>>>> +                         size_int (dirprec)))))
>>>> +  {
>>>> +    *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0,
>>>> false);
>>>> +    *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0,
>>>> false);
>>>> +    return false;
>>>> +  }
>>> So in this case we're not changing the values, we're just converting
>>> them to a equal or wider type, right?  Thus no adjustment (what about a
>>> sign change?  Is that an adjustment?) and we return false per the
>>> function's specifications.
>>
>> This casts the value to the destination type, so it may result in
>> different values.  The important postcondition it preserves is that
>> the difference ARGMAX - ARGMIN is less than or equal to TYPE_MAX of
>> the directive's unsigned TYPE.  (If it isn't, the range cannot be
>> converted.)  This lets us take, say:
>>
>>   int f (int i)
>>   {
>>     if (i < 1024 || 1033 < i)
>>        i = 1024;
>>
>>     return snprintf (0, 0, "%hhi", i);
>>   }
>>
>> and fold the function call into 1 because (signed char)1024 is 0 and
>> (signed char)1033 is 9, and every other value in that same original
>> range is also in the same range after the conversion.  We know it's
>> safe because (1033 - 1024 <= UCHAR_MAX) holds.
> But the code in question is guarded by dirprec >= argprec.  Thus aren't
> we converting to an equal or wider type?  In the case of converting to a
> wider type, ISTM the values won't change and thus we return false.

I just saw your other response after I typed this up (thanks!) but
so just to close the loop:

You're right, when converting to a wider type the values won't change.

The guard is an OR expression:

       dirprec >= argprec
   OR
       0 == ((ARGMAX - ARGMIN) >> TYPE_PRECISION (dirtype))

and so when (dirprec < argprec) holds the values might change if
the other operand is true.  In both of these cases the function
returns false to indicate that the original range the arguments
were in hasn't changed as a result.

>
> If we are converting to the same sized type, but with a different
> signedness, then the values will have been adjusted and ISTM we ought to
> be returning true in that case.  But the code actually returns false.
>
> I must be missing something here.

The return value of the function is only used to decide the phrasing
of the notes printed after warnings and what values to include in
them.  Returning false means that the notes will say something like
"directive argument in the range [ARGMIN, ARGMAX]" where the values
are those determined by VRP.  Returning true means that the note
will instead say "using the range [TYPE_MIN, 0] for directive
argument." This distinction will (I hope) let the user know that
the second case is less accurate than the first.  It's somewhat
subtle but I think useful to understand why the checker decided
to warn.  Once I'm done with all my work I'd like to document this
in more  detail on the Wiki.


>>> What about overflows when either argmin or argmax won't fit into dirtype
>>> without an overflow?  What's the right behavior there?
>>
>> That's fine just as long as the property above holds.
>>
>> I think the algorithm works but the code came from tree-vrp where
>> there are all sorts of additional checks for some infinities which
>> VRP distinguishes from type limits like SCHAR_MIN and _MAX.  I don't
>> fully understand those and so I'd feel better if someone who does
>> double checked this code.
> That's what prompted my question about overflows.  It was a general
> concern after reading the VRP code.  I did not have a specific case in
> mind that would be mis-handled.

Okay.

>>>> +  tree dirmin = TYPE_MIN_VALUE (dirtype);
>>>> +  tree dirmax = TYPE_MAX_VALUE (dirtype);
>>> From this point onward argmin/argmax are either not integers or we're
>>> doing a narrowing conversion, right?  In both cases the fact that we're
>>> doing a narrowing conversion constrains the range
>>
>> Argmin and argmax are always integers.  The rest of the function
>> handles the case where the postcondition above doesn't hold (e.g.,
>> in function g above).
> OK.  So is the hangup really a problem in how the return type is
> documented?  I parsed the comment as essentially saying we return true
> if the range gets adjusted in any way -- thus a sign change in the first
> block would qualify, but we returned false which seemed inconsistent.
>
> Looking at it again, what I think it's saying is we're returning true
> only for a subset of adjustments to the range, ie, those cases where the
> postcondition does not hold.  Correct?

Correct.  Would changing the description of the return value to
this make it clearer?

    Return true when the range has been adjusted to the full unsigned
    range of DIRTYPE, [0, DIRTYPE_MAX], false otherwise.

Martin

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

> So I think the return value needs a bit of clarification here.  Take
> guidance from our discussion on this thread.
>
> OK with that fixed.
>
> jeff

The "strange test failures​" that I wrote about in a separate thread
late last week prompted me to re-review the patch and add more test
cases.  Those in turn exposed a bug in the adjust_range_for_overflow
function involving types of the same precision but different sign
where converting an unsigned range with an upper bound in excess of
the directive's TYPE_MAX would incorrectly accept the converted range
even though the new upper bound was less than the lower bound.

The updated  patch corrects this oversight.  In addition, it adjusts
the handling of the obscure corner case of zero precision and zero
argument which results in zero bytes (except in some even more
obscure cases involving some flags for some conversions).  For
instance:

   printf ("%.0i", 0);

results in zero bytes, but

   printf ("%+.0i", 0);

results in 1 byte (and prints '+').  This is tracked in bug 78606.

Although the differences between the approved patch and the update
are very small I repost it in case one of you would like to double
check them.  If not I'll commit the updated patch later in the week.

Martin

[-- Attachment #2: gcc-78622.diff --]
[-- Type: text/x-patch, Size: 61461 bytes --]

PR middle-end/78622 - -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping
PR middle-end78606 - -Wformat-length/-fprintf-return-value incorrect for %+.0i and %.0o with zero value

gcc/ChangeLog:

	PR middle-end/78622
	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
	rather than res.bounded.
	(get_width_and_precision): Set precision to -1 when negative.
	(adjust_range_for_overflow): New function.
	(format_integer): Correct the handling of the space, plus, and pound
	flags, and the special case of zero precision.
	Always set res.bounded to true unless either precision or width
	is specified and unknown.
	Call adjust_range_for_overflow.
	Avoid use zero as the shortest value when precision is specified
	but unknown.
	(format_directive): Remove vestigial quoting.  Always inform of
	argument value or range when it's available.
	(add_bytes): Correct the computation of boundrange used to
	decide whether a warning is of a "maybe" or "defnitely" kind.

gcc/testsuite/ChangeLog:

	PR middle-end/78622
	* gcc.c-torture/execute/pr78622.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
	behavior inadvertently introduced in a previous commit.  Tighten
	up final checking.
	* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
	Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-2.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
	add a final optimization check.
	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.
	* gcc.dg/tree-ssa/pr78622.c: New test.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 99a635a..905917d 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -637,7 +637,7 @@ min_bytes_remaining (unsigned HOST_WIDE_INT navail, const format_result &res)
   if (HOST_WIDE_INT_MAX <= navail)
     return navail;
 
-  if (1 < warn_format_length || res.bounded)
+  if (1 < warn_format_length || res.knownrange)
     {
       /* At level 2, or when all directives output an exact number
 	 of bytes or when their arguments were bounded by known
@@ -785,7 +785,7 @@ get_width_and_precision (const conversion_spec &spec,
 	{
 	  prec = tree_to_shwi (spec.star_precision);
 	  if (prec < 0)
-	    prec = 0;
+	    prec = -1;
 	}
       else
 	prec = HOST_WIDE_INT_MIN;
@@ -795,6 +795,69 @@ get_width_and_precision (const conversion_spec &spec,
   *pprec = prec;
 }
 
+/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
+   argument, due to the conversion from either *ARGMIN or *ARGMAX to
+   the type of the directive's formal argument it's possible for both
+   to result in the same number of bytes or a range of bytes that's
+   less than the number of bytes that would result from formatting
+   some other value in the range [*ARGMIN, *ARGMAX].  This can be
+   determined by checking for the actual argument being in the range
+   of the type of the directive.  If it isn't it must be assumed to
+   take on the full range of the directive's type.
+   Return true when the range has been adjusted to the full unsigned
+   range of DIRTYPE, or [0, DIRTYPE_MAX], and false otherwise.  */
+
+static bool
+adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
+{
+  tree argtype = TREE_TYPE (*argmin);
+  unsigned argprec = TYPE_PRECISION (argtype);
+  unsigned dirprec = TYPE_PRECISION (dirtype);
+
+  /* If the actual argument and the directive's argument have the same
+     precision and sign there can be no overflow and so there is nothing
+     to adjust.  */
+  if (argprec == dirprec && TYPE_SIGN (argtype) == TYPE_SIGN (dirtype))
+    return false;
+
+  /* The logic below was inspired/lifted from the CONVERT_EXPR_CODE_P
+     branch in the extract_range_from_unary_expr function in tree-vrp.c.  */
+
+  if (TREE_CODE (*argmin) == INTEGER_CST
+      && TREE_CODE (*argmax) == INTEGER_CST
+      && (dirprec >= argprec
+	  || integer_zerop (int_const_binop (RSHIFT_EXPR,
+					     int_const_binop (MINUS_EXPR,
+							      *argmax,
+							      *argmin),
+					     size_int (dirprec)))))
+    {
+      *argmin = force_fit_type (dirtype, wi::to_widest (*argmin), 0, false);
+      *argmax = force_fit_type (dirtype, wi::to_widest (*argmax), 0, false);
+
+      /* If *ARGMIN is still less than *ARGMAX the conversion above
+	 is safe.  Otherwise, it has overflowed and would be unsafe.  */
+      if (tree_int_cst_le (*argmin, *argmax))
+	return false;
+    }
+
+  tree dirmin = TYPE_MIN_VALUE (dirtype);
+  tree dirmax = TYPE_MAX_VALUE (dirtype);
+
+  if (TYPE_UNSIGNED (dirtype))
+    {
+      *argmin = dirmin;
+      *argmax = dirmax;
+    }
+  else
+    {
+      *argmin = integer_zero_node;
+      *argmax = dirmin;
+    }
+
+  return true;
+}
+
 /* Return a range representing the minimum and maximum number of bytes
    that the conversion specification SPEC will write on output for the
    integer argument ARG when non-null.  ARG may be null (for vararg
@@ -886,27 +949,24 @@ format_integer (const conversion_spec &spec, tree arg)
       int base;
 
       /* True when a signed conversion is preceded by a sign or space.  */
-      bool maybesign;
+      bool maybesign = false;
 
       switch (spec.specifier)
 	{
 	case 'd':
 	case 'i':
-	  /* Space is only effective for signed conversions.  */
-	  maybesign = spec.get_flag (' ');
+	  /* Space and '+' are  only meaningful for signed conversions.  */
+	  maybesign = spec.get_flag (' ') | spec.get_flag ('+');
 	  base = 10;
 	  break;
 	case 'u':
-	  maybesign = spec.force_flags ? spec.get_flag (' ') : false;
 	  base = 10;
 	  break;
 	case 'o':
-	  maybesign = spec.force_flags ? spec.get_flag (' ') : false;
 	  base = 8;
 	  break;
 	case 'X':
 	case 'x':
-	  maybesign = spec.force_flags ? spec.get_flag (' ') : false;
 	  base = 16;
 	  break;
 	default:
@@ -917,20 +977,20 @@ format_integer (const conversion_spec &spec, tree arg)
 
       if ((prec == HOST_WIDE_INT_MIN || prec == 0) && integer_zerop (arg))
 	{
-	  /* As a special case, a precision of zero with an argument
-	     of zero results in zero bytes regardless of flags (with
-	     width having the normal effect).  This must extend to
-	     the case of a specified precision with an unknown value
-	     because it can be zero.  */
-	  len = 0;
+	  /* As a special case, a precision of zero with a zero argument
+	     results in zero bytes except in base 8 when the '#' flag is
+	     specified, and for signed conversions in base 8 and 10 when
+	     flags when either the space or '+' flag has been specified
+	     when it results in just one byte (with width having the normal
+	     effect).  This must extend to the case of a specified precision
+	     with an unknown value because it can be zero.  */
+	  len = ((base == 8 && spec.get_flag ('#')) || maybesign);
 	}
       else
 	{
 	  /* Convert the argument to the type of the directive.  */
 	  arg = fold_convert (dirtype, arg);
 
-	  maybesign |= spec.get_flag ('+');
-
 	  /* True when a conversion is preceded by a prefix indicating the base
 	     of the argument (octal or hexadecimal).  */
 	  bool maybebase = spec.get_flag ('#');
@@ -989,6 +1049,10 @@ format_integer (const conversion_spec &spec, tree arg)
 
   fmtresult res;
 
+  /* The result is bounded unless width or precision has been specified
+     whose value is unknown.  */
+  res.bounded = width != HOST_WIDE_INT_MIN && prec != HOST_WIDE_INT_MIN;
+
   /* Using either the range the non-constant argument is in, or its
      type (either "formal" or actual), create a range of values that
      constrain the length of output given the warning level.  */
@@ -1005,38 +1069,19 @@ format_integer (const conversion_spec &spec, tree arg)
       enum value_range_type range_type = get_range_info (arg, &min, &max);
       if (range_type == VR_RANGE)
 	{
-	  res.argmin = build_int_cst (argtype, wi::fits_uhwi_p (min)
-				      ? min.to_uhwi () : min.to_shwi ());
-	  res.argmax = build_int_cst (argtype, wi::fits_uhwi_p (max)
-				      ? max.to_uhwi () : max.to_shwi ());
-
-	  /* For a range with a negative lower bound and a non-negative
-	     upper bound, use one to determine the minimum number of bytes
-	     on output and whichever of the two bounds that results in
-	     the greater number of bytes on output for the upper bound.
-	     For example, for ARG in the range of [-3, 123], use 123 as
-	     the upper bound for %i but -3 for %u.  */
-	  if (wi::neg_p (min) && !wi::neg_p (max))
-	    {
-	      argmin = res.argmin;
-	      argmax = res.argmax;
-	      int minbytes = format_integer (spec, res.argmin).range.min;
-	      int maxbytes = format_integer (spec, res.argmax).range.max;
-	      if (maxbytes < minbytes)
-		argmax = res.argmin;
-
-	      argmin = integer_zero_node;
-	    }
-	  else
-	    {
-	      argmin = res.argmin;
-	      argmax = res.argmax;
-	    }
-
-	  /* The argument is bounded by the known range of values
-	     determined by Value Range Propagation.  */
-	  res.bounded = true;
-	  res.knownrange = true;
+	  argmin = build_int_cst (argtype, wi::fits_uhwi_p (min)
+				  ? min.to_uhwi () : min.to_shwi ());
+	  argmax = build_int_cst (argtype, wi::fits_uhwi_p (max)
+				  ? max.to_uhwi () : max.to_shwi ());
+
+	  /* Set KNOWNRANGE if the argument is in a known subrange
+	     of the directive's type (KNOWNRANGE may be reset below).  */
+	  res.knownrange
+	    = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), argmin)
+	       || !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), argmax));
+
+	  res.argmin = argmin;
+	  res.argmax = argmax;
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
@@ -1073,7 +1118,7 @@ format_integer (const conversion_spec &spec, tree arg)
 	 can output.  When precision is specified but unknown, use zero
 	 as the minimum since it results in no bytes on output (unless
 	 width is specified to be greater than 0).  */
-      argmin = build_int_cst (argtype, prec != HOST_WIDE_INT_MIN);
+      argmin = build_int_cst (argtype, prec && prec != HOST_WIDE_INT_MIN);
 
       int typeprec = TYPE_PRECISION (dirtype);
       int argprec = TYPE_PRECISION (argtype);
@@ -1101,11 +1146,46 @@ format_integer (const conversion_spec &spec, tree arg)
       res.argmax = argmax;
     }
 
+  if (tree_int_cst_lt (argmax, argmin))
+    {
+      tree tmp = argmax;
+      argmax = argmin;
+      argmin = tmp;
+    }
+
+  /* Clear KNOWNRANGE if the range has been adjusted to the maximum
+     of the directive.  If it has been cleared then since ARGMIN and/or
+     ARGMAX have been adjusted also adjust the corresponding ARGMIN and
+     ARGMAX in the result to include in diagnostics.  */
+  if (adjust_range_for_overflow (dirtype, &argmin, &argmax))
+    {
+      res.knownrange = false;
+      res.argmin = argmin;
+      res.argmax = argmax;
+    }
+
   /* Recursively compute the minimum and maximum from the known range,
      taking care to swap them if the lower bound results in longer
      output than the upper bound (e.g., in the range [-1, 0].  */
-  res.range.min = format_integer (spec, argmin).range.min;
-  res.range.max = format_integer (spec, argmax).range.max;
+
+  if (TYPE_UNSIGNED (dirtype))
+    {
+      /* For unsigned conversions/directives, use the minimum (i.e., 0
+	 or 1) and maximum to compute the shortest and longest output,
+	 respectively.  */
+      res.range.min = format_integer (spec, argmin).range.min;
+      res.range.max = format_integer (spec, argmax).range.max;
+    }
+  else
+    {
+      /* For signed conversions/directives, use the maximum (i.e., 0)
+	 to compute the shortest output and the minimum (i.e., TYPE_MIN)
+	 to compute the longest output.  This is important when precision
+	 is specified but unknown because otherwise both output lengths
+	 would reflect the largest possible precision (i.e., INT_MAX).  */
+      res.range.min = format_integer (spec, argmax).range.min;
+      res.range.max = format_integer (spec, argmin).range.max;
+    }
 
   /* The result is bounded either when the argument is determined to be
      (e.g., when it's within some range) or when the minimum and maximum
@@ -1776,13 +1856,13 @@ format_directive (const pass_sprintf_length::call_info &info,
      NUL that's appended after the format string has been processed.  */
   unsigned HOST_WIDE_INT navail = min_bytes_remaining (info.objsize, *res);
 
+  bool warned = res->warned;
+
   if (fmtres.range.min < fmtres.range.max)
     {
       /* The result is a range (i.e., it's inexact).  */
-      if (!res->warned)
+      if (!warned)
 	{
-	  bool warned = false;
-
 	  if (navail < fmtres.range.min)
 	    {
 	      /* The minimum directive output is longer than there is
@@ -1867,21 +1947,6 @@ format_directive (const pass_sprintf_length::call_info &info,
 				    navail);
 		}
 	    }
-
-	  res->warned |= warned;
-
-	  if (warned && fmtres.argmin)
-	    {
-	      if (fmtres.argmin == fmtres.argmax)
-		inform (info.fmtloc, "directive argument %qE", fmtres.argmin);
-	      else if (fmtres.bounded)
-		inform (info.fmtloc, "directive argument in the range [%E, %E]",
-			fmtres.argmin, fmtres.argmax);
-	      else
-		inform (info.fmtloc,
-			"using the range [%qE, %qE] for directive argument",
-			fmtres.argmin, fmtres.argmax);
-	    }
 	}
 
       /* Disable exact length checking but adjust the minimum and maximum.  */
@@ -1894,7 +1959,7 @@ format_directive (const pass_sprintf_length::call_info &info,
     }
   else
     {
-      if (!res->warned && fmtres.range.min > 0 && navail < fmtres.range.min)
+      if (!warned && fmtres.range.min > 0 && navail < fmtres.range.min)
 	{
 	  const char* fmtstr
 	    = (info.bounded
@@ -1909,10 +1974,10 @@ format_directive (const pass_sprintf_length::call_info &info,
 		  : G_("%<%.*s%> directive writing %wu byte "
 		       "into a region of size %wu")));
 
-	  res->warned = fmtwarn (dirloc, pargrange, NULL,
-				 OPT_Wformat_length_, fmtstr,
-				 (int)cvtlen, cvtbeg, fmtres.range.min,
-				 navail);
+	  warned = fmtwarn (dirloc, pargrange, NULL,
+			    OPT_Wformat_length_, fmtstr,
+			    (int)cvtlen, cvtbeg, fmtres.range.min,
+			    navail);
 	}
       *res += fmtres.range.min;
     }
@@ -1923,7 +1988,7 @@ format_directive (const pass_sprintf_length::call_info &info,
   if (!minunder4k || fmtres.range.max > 4095)
     res->under4k = false;
 
-  if (!res->warned && 1 < warn_format_length
+  if (!warned && 1 < warn_format_length
       && (!minunder4k || fmtres.range.max > 4095))
     {
       /* The directive output may be longer than the maximum required
@@ -1934,11 +1999,11 @@ format_directive (const pass_sprintf_length::call_info &info,
 	 (like Glibc does under some conditions).  */
 
       if (fmtres.range.min == fmtres.range.max)
-	res->warned = fmtwarn (dirloc, pargrange, NULL,
-			       OPT_Wformat_length_,
-			       "%<%.*s%> directive output of %wu bytes exceeds "
-			       "minimum required size of 4095",
-			       (int)cvtlen, cvtbeg, fmtres.range.min);
+	warned = fmtwarn (dirloc, pargrange, NULL,
+			  OPT_Wformat_length_,
+			  "%<%.*s%> directive output of %wu bytes exceeds "
+			  "minimum required size of 4095",
+			  (int)cvtlen, cvtbeg, fmtres.range.min);
       else
 	{
 	  const char *fmtstr
@@ -1948,17 +2013,17 @@ format_directive (const pass_sprintf_length::call_info &info,
 	       : G_("%<%.*s%> directive output between %qu and %wu "
 		    "bytes exceeds minimum required size of 4095"));
 
-	  res->warned = fmtwarn (dirloc, pargrange, NULL,
-				 OPT_Wformat_length_, fmtstr,
-				 (int)cvtlen, cvtbeg,
-				 fmtres.range.min, fmtres.range.max);
+	  warned = fmtwarn (dirloc, pargrange, NULL,
+			    OPT_Wformat_length_, fmtstr,
+			    (int)cvtlen, cvtbeg,
+			    fmtres.range.min, fmtres.range.max);
 	}
     }
 
   /* Has the minimum directive output length exceeded INT_MAX?  */
   bool exceedmin = res->number_chars_min > target_int_max ();
 
-  if (!res->warned
+  if (!warned
       && (exceedmin
 	  || (1 < warn_format_length
 	      && res->number_chars_max > target_int_max ())))
@@ -1967,11 +2032,11 @@ format_directive (const pass_sprintf_length::call_info &info,
 	 to exceed INT_MAX bytes.  */
 
       if (fmtres.range.min == fmtres.range.max)
-	res->warned = fmtwarn (dirloc, pargrange, NULL,
-			       OPT_Wformat_length_,
-			       "%<%.*s%> directive output of %wu bytes causes "
-			       "result to exceed %<INT_MAX%>",
-			       (int)cvtlen, cvtbeg, fmtres.range.min);
+	warned = fmtwarn (dirloc, pargrange, NULL,
+			  OPT_Wformat_length_,
+			  "%<%.*s%> directive output of %wu bytes causes "
+			  "result to exceed %<INT_MAX%>",
+			  (int)cvtlen, cvtbeg, fmtres.range.min);
       else
 	{
 	  const char *fmtstr
@@ -1980,12 +2045,27 @@ format_directive (const pass_sprintf_length::call_info &info,
 		     "bytes causes result to exceed %<INT_MAX%>")
 	       : G_ ("%<%.*s%> directive output between %wu and %wu "
 		     "bytes may cause result to exceed %<INT_MAX%>"));
-	  res->warned = fmtwarn (dirloc, pargrange, NULL,
-				 OPT_Wformat_length_, fmtstr,
-				 (int)cvtlen, cvtbeg,
-				 fmtres.range.min, fmtres.range.max);
+	  warned = fmtwarn (dirloc, pargrange, NULL,
+			    OPT_Wformat_length_, fmtstr,
+			    (int)cvtlen, cvtbeg,
+			    fmtres.range.min, fmtres.range.max);
 	}
     }
+
+  if (warned && fmtres.argmin)
+    {
+      if (fmtres.argmin == fmtres.argmax)
+	inform (info.fmtloc, "directive argument %qE", fmtres.argmin);
+      else if (fmtres.knownrange)
+	inform (info.fmtloc, "directive argument in the range [%E, %E]",
+		fmtres.argmin, fmtres.argmax);
+      else
+	inform (info.fmtloc,
+		"using the range [%E, %E] for directive argument",
+		fmtres.argmin, fmtres.argmax);
+    }
+
+  res->warned |= warned;
 }
 
 /* Account for the number of bytes between BEG and END (or between
@@ -2057,7 +2137,7 @@ add_bytes (const pass_sprintf_length::call_info &info,
 	 indicate that the overflow/truncation may (but need not) happen.  */
       bool boundrange
 	= (res->number_chars_min < res->number_chars_max
-	   && res->number_chars_min < info.objsize);
+	   && res->number_chars_min + nbytes <= info.objsize);
 
       if (!end && ((nbytes - navail) == 1 || boundrange))
 	{
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78622.c b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
new file mode 100644
index 0000000..d835c74
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr78622.c
@@ -0,0 +1,34 @@
+/* PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value
+   incorrect with overflow/wrapping
+   { dg-additional-options "-Wformat-length=2" } */
+
+__attribute__((noinline, noclone)) int
+foo (int x)
+{
+  if (x < 4096 + 8 || x >= 4096 + 256 + 8)
+    return -1;
+
+  char buf[5];
+  int n = __builtin_snprintf (buf, sizeof buf, "%hhd", x + 1);
+  __builtin_printf ("\"%hhd\" => %i\n", x + 1, n);
+  return n;
+}
+
+int
+main (void)
+{
+  if (__SCHAR_MAX__ != 127 || __CHAR_BIT__ != 8 || __SIZEOF_INT__ != 4)
+    return 0;
+
+  if (foo (4095 + 9) != 1
+      || foo (4095 + 32) != 2
+      || foo (4095 + 127) != 3
+      || foo (4095 + 128) != 4
+      || foo (4095 + 240) != 3
+      || foo (4095 + 248) != 2
+      || foo (4095 + 255) != 2
+      || foo (4095 + 256) != 1)
+    __builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
index 4dddccdf..260f4fc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-2.c
@@ -25,6 +25,9 @@ char buf8k [8192];
 #define concat(a, b)   a ## b
 #define CAT(a, b)      concat (a, b)
 
+/* Calls to this function must not be eliminated.  */
+void must_not_eliminate (void);
+
 #define EQL(expect, size, fmt, ...)					\
   void __attribute__ ((noinline, noclone))				\
   CAT (test_on_line_, __LINE__)(void)					\
@@ -34,7 +37,7 @@ char buf8k [8192];
 	char *dst = size < 0 ? buf : buf8k + sizeof buf8k - size;	\
 	int result = __builtin_sprintf (dst, fmt, __VA_ARGS__);		\
 	if (result != expect)						\
-	  __builtin_abort ();						\
+	  must_not_eliminate ();					\
       }									\
   }
 
@@ -50,7 +53,7 @@ char buf8k [8192];
 	char *dst = size < 0 ? buf : buf8k + sizeof buf8k - size;	\
 	int result = __builtin_sprintf (dst, fmt, __VA_ARGS__);		\
 	if (result < min || max < result)				\
-	  __builtin_abort ();						\
+	  must_not_eliminate ();					\
       }									\
   }
 
@@ -75,6 +78,8 @@ EQL (0, 0, "%-s", "");
 EQL (1, 1, "%c",  'x');
 EQL (1, 1, "%-s", "x");
 
+EQL (1, 2, "%c",  'x');
+
 EQL (4, 4, "%4c", 'x');
 
 /* Verify that exceeding the environmental limit of 4095 bytes for
@@ -179,7 +184,7 @@ RNG (4,  4, 32, "%zu", sz)
 
 /* Exercise bug 78586.  */
 RNG (4,  4, 32, "%lu", (unsigned long)i)
-RNG (4,  4, 32, "%lu", (unsigned)u)
+RNG (4,  4, 32, "%lu", (unsigned long)u)
 RNG (4,  4, 32, "%lu", (unsigned long)li)
 RNG (4,  4, 32, "%lu", (unsigned long)lu)
 RNG (4,  4, 32, "%lu", (unsigned long)sz)
@@ -259,21 +264,24 @@ RNG (0,  6,   8, "%s%ls", "1", L"2");
 /* Verify that no call to abort has been eliminated and that each call
    is at the beginning of a basic block (and thus the result of a branch).
    This latter test tries to verify that the test preceding the call to
-   abort has not been eliminated either.
+   the must_not_eliminate() function has not been eliminated either.
 
    The expected output looks something like this:
 
    <bb 2>:
    result_3 = __builtin_sprintf (&MEM[(void *)&buf8k + 8192B], "%c", 32);
    if (result_3 != 0)
-     goto <bb 3>;
+     goto <bb 3>; [50.0%]
    else
-     goto <bb 4>;
+     goto <bb 4>; [50.0%]
 
-   <bb 3>:
-   __builtin_abort ();
+   <bb 3>[50.0%]:
+   must_not_eliminate ();
 
 */
 
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 124 "optimized" { target { ilp32 || lp64 } } } } */
-/* { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *__builtin_abort" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } } */
+/*  Only conditional calls to abort should be made (with any probability):
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *must_not_eliminate" 124 "optimized" { target { ilp32 || lp64 } } } }
+    { dg-final { scan-tree-dump-times "> \\\[\[0-9.\]+%\\\]:\n *must_not_eliminate" 93 "optimized" { target { { ! ilp32 } && { ! lp64 } } } } }
+    No unconditional calls to abort should be made:
+    { dg-final { scan-tree-dump-not ";\n *must_not_eliminate" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
index dbb0dd9..c4489ac 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-5.c
@@ -8,18 +8,20 @@
 #define FAIL(line)  CAT (failure_on_line_, line)
 #define PASS(line)  CAT (success_on_line_, line)
 
-/* Emit a call to a function named failure_on_line_NNN when EXPR is false.  */
-#define ASSERT(value, expect)			\
+/* Emit a call to a function named failure_on_line_NNN when VALUE
+   is not equal to the constant EXPECTED, otherwise emit a call to
+   function success_on_line_NNN.  */
+#define ASSERT(value, expected)			\
   do {						\
     extern void FAIL (__LINE__)(int);		\
     extern void PASS (__LINE__)(int);		\
-    if (value == expect)			\
+    if (value == expected)			\
       PASS (__LINE__)(value);			\
     else					\
       FAIL (__LINE__)(value);			\
   } while (0)
 
-#define T(expect, ...)					\
+#define EQL(expect, ...)				\
   do {							\
     int n = __builtin_snprintf (0, 0, __VA_ARGS__);	\
     ASSERT (n, expect);					\
@@ -27,88 +29,118 @@
 
 int ival (int i) { return i; }
 
+/* Generate a signed int value in the specified range.  */
+
+static int
+int_range (int min, int max)
+{
+  extern int int_value (void);
+  int val = int_value ();
+  if (val < min || max < val)
+    val = min;
+  return val;
+}
+
+#define R(min, max) int_range (min, max)
+
 void test_arg_int (int i, int n)
 {
-  T (1, "%i", ival (0));
-  T (1, "%i", ival (1));
-  T (2, "%i%i", ival (0), ival (1));
-  T (3, "%i%i%i", ival (0), ival (1), ival (9));
-  T (5, "%i %i %i", ival (0), ival (1), ival (9));
+  EQL (1, "%i", ival (0));
+  EQL (1, "%i", ival (1));
+  EQL (2, "%i%i", ival (0), ival (1));
+  EQL (3, "%i%i%i", ival (0), ival (1), ival (9));
+  EQL (5, "%i %i %i", ival (0), ival (1), ival (9));
 
-  T (5, "%i %i %i", ival (0), ival (1), ival (9));
+  EQL (5, "%i %i %i", ival (0), ival (1), ival (9));
 
-  T (13, "%hhu.%hhu.%hhu.%hhu", ival (23), ival (78), ival (216), ival (135));
+  EQL (13, "%hhu.%hhu.%hhu.%hhu", ival (23), ival (78), ival (216), ival (135));
 
   for (i = 0; i != 9; ++i)
-    T (1, "%i", i);
+    EQL (1, "%i", i);
 
   for (i = -n; i != n; ++i)
-    T (8, "%08x", i);
+    EQL (8, "%08x", i);
 
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
-  T (0, "%.0d", ival (0));
-  T (0, "%.0i", ival (0));
-  T (0, "%.0o", ival (0));
-  T (0, "%.0u", ival (0));
-  T (0, "%.0x", ival (0));
-
-  T (0, "%.*d", 0, ival (0));
-  T (0, "%.*i", 0, ival (0));
-  T (0, "%.*o", 0, ival (0));
-  T (0, "%.*u", 0, ival (0));
-  T (0, "%.*x", 0, ival (0));
-
-  T (1, "%1.0d", ival (0));
-  T (1, "%1.0i", ival (0));
-  T (1, "%1.0o", ival (0));
-  T (1, "%1.0u", ival (0));
-  T (1, "%1.0x", ival (0));
+  EQL (0, "%.0d", ival (0));
+  EQL (0, "%.0i", ival (0));
+  EQL (0, "%.0o", ival (0));
+  EQL (0, "%.0u", ival (0));
+  EQL (0, "%.0x", ival (0));
+
+  EQL (0, "%.*d", 0, ival (0));
+  EQL (0, "%.*i", 0, ival (0));
+  EQL (0, "%.*o", 0, ival (0));
+  EQL (0, "%.*u", 0, ival (0));
+  EQL (0, "%.*x", 0, ival (0));
+
+  EQL (1, "%1.0d", ival (0));
+  EQL (1, "%1.0i", ival (0));
+  EQL (1, "%1.0o", ival (0));
+  EQL (1, "%1.0u", ival (0));
+  EQL (1, "%1.0x", ival (0));
+
+  EQL (4, "%hhi", R (-128, -127));
+  EQL (3, "%hhi", R ( -99,  -10));
+  EQL (2, "%hhi", R (  -9,   -1));
+  EQL (1, "%hhi", R (   0,    9));
+  EQL (1, "%hhi", R (   0,    9));
+
+  EQL (1, "%1.0hhi", R (   0,    1));
+  EQL (1, "%1.1hhi", R (   0,    9));
+  EQL (2, "%1.2hhi", R (   0,    9));
+  EQL (3, "%1.3hhi", R (   0,    9));
+
+  EQL (1, "%hhi", R (1024, 1033));
+  EQL (2, "%hhi", R (1034, 1123));
+  EQL (1, "%hhu", R (1024, 1033));
+  EQL (2, "%hhu", R (1034, 1123));
 }
 
 void test_arg_string (const char *s)
 {
-  T ( 0, "%-s", "");
-  T ( 1, "%%");
-  T ( 1, "%-s", "1");
-  T ( 2, "%-s", "12");
-  T ( 3, "%-s", "123");
-  T ( 5, "s=%s", "123");
-  T (10, "%%s=\"%s\"", "12345");
-
-  T ( 1, "%.*s", 1, "123");
-  T ( 2, "%.*s", 2, "123");
-  T ( 3, "%.*s", 3, "123");
-  T ( 3, "%.*s", 4, "123");
-
-  T ( 1, "%1.*s", 1, "123");
-  T ( 2, "%1.*s", 2, "123");
-  T ( 3, "%1.*s", 3, "123");
-  T ( 3, "%1.*s", 4, "123");
-  T ( 4, "%4.*s", 1, "123");
-  T ( 4, "%4.*s", 2, "123");
-  T ( 4, "%4.*s", 3, "123");
-  T ( 4, "%4.*s", 4, "123");
-  T ( 4, "%4.*s", 5, "123");
+  EQL ( 0, "%-s", "");
+  EQL ( 1, "%%");
+  EQL ( 1, "%-s", "1");
+  EQL ( 2, "%-s", "12");
+  EQL ( 3, "%-s", "123");
+  EQL ( 5, "s=%s", "123");
+  EQL (10, "%%s=\"%s\"", "12345");
+
+  EQL ( 1, "%.*s", 1, "123");
+  EQL ( 2, "%.*s", 2, "123");
+  EQL ( 3, "%.*s", 3, "123");
+  EQL ( 3, "%.*s", 4, "123");
+
+  EQL ( 1, "%1.*s", 1, "123");
+  EQL ( 2, "%1.*s", 2, "123");
+  EQL ( 3, "%1.*s", 3, "123");
+  EQL ( 3, "%1.*s", 4, "123");
+  EQL ( 4, "%4.*s", 1, "123");
+  EQL ( 4, "%4.*s", 2, "123");
+  EQL ( 4, "%4.*s", 3, "123");
+  EQL ( 4, "%4.*s", 4, "123");
+  EQL ( 4, "%4.*s", 5, "123");
 
   const char *a = "123";
   const char *b = "456";
 
-  T ( 3, "%-s", s ? a : b);
-  T ( 0, "%.0s", s);
-  T ( 1, "%1.1s", s);
-  T ( 2, "%2.2s", s);
-  T ( 2, "%2.1s", s);
+  EQL ( 3, "%-s", s ? a : b);
+  EQL ( 0, "%.0s", s);
+  EQL ( 1, "%1.1s", s);
+  EQL ( 2, "%2.2s", s);
+  EQL ( 2, "%2.1s", s);
 }
 
 void test_arg_multiarg (int i, double d)
 {
-  T (16, "%i %f %s", 123, 3.14, "abc");
-  T (16, "%12i %s", i, "abc");
-  T (16, "%*i %s", 12, i, "abc");
+  EQL (16, "%i %f %s", 123, 3.14, "abc");
+  EQL (16, "%12i %s", i, "abc");
+  EQL (16, "%*i %s", 12, i, "abc");
 }
 
-#define TV(expect, fmt, va)				\
+#define EQLv(expect, fmt, va)				\
   do {							\
     int n = __builtin_vsnprintf (0, 0, fmt, va);	\
     ASSERT (n, expect);					\
@@ -116,21 +148,21 @@ void test_arg_multiarg (int i, double d)
 
 void test_va_int (__builtin_va_list va)
 {
-  TV ( 2, "%02hhx", va);
-  TV ( 2, "%02.*hhx", va);
-  TV ( 4, "%04hx", va);
-  TV ( 4, "%04.*hx", va);
+  EQLv ( 2, "%02hhx", va);
+  EQLv ( 2, "%02.*hhx", va);
+  EQLv ( 4, "%04hx", va);
+  EQLv ( 4, "%04.*hx", va);
 }
 
 void test_va_multiarg (__builtin_va_list va)
 {
-  TV ( 8, "%8x", va);
-  TV ( 8, "% 8x", va);
-  TV ( 9, "%9x", va);
-  TV (11, "%11o", va);
-  TV (12, "%12o", va);
+  EQLv ( 8, "%8x", va);
+  EQLv ( 8, "% 8x", va);
+  EQLv ( 9, "%9x", va);
+  EQLv (11, "%11o", va);
+  EQLv (12, "%12o", va);
 
-  TV (16, "%12i %3.2s", va);
+  EQLv (16, "%12i %3.2s", va);
 }
 
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
index 4c41234..abd49df 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
@@ -8,6 +8,8 @@
    { dg-options "-O2 -Wformat -fdump-tree-optimized" }
    { dg-require-effective-target int32plus } */
 
+typedef __SIZE_TYPE__ size_t;
+
 #define CONCAT(a, b) a ## b
 #define CAT(a, b)    CONCAT (a, b)
 
@@ -50,6 +52,19 @@ void test_arg_int (int width, int prec, int i, int n)
 
   T ("%i", R (1, 10));
 
+  /* Each of the bounds of the ranges below results in just one byte
+     on output because they convert to the value 9 in type char, yet
+     other values in those ranges can result in up to four bytes.
+     For example, 4240 converts to -112.  Verify that the output
+     isn't folded into a constant.  This assumes __CHAR_BIT__ of 8.  */
+  T ("%hhi", R (4104, 4360) + 1);
+  T ("%hhu", R (4104, 4360) + 1);
+
+  /* Here, the range includes all possible lengths of output for
+     a 16-bit short and 32-bit int.  */
+  T ("%hi", R (65536, 65536 * 2));
+  T ("%hu", R (65536, 65536 * 2));
+
   T ("%'i", 1234567);
 
   for (i = -n; i != n; ++i)
@@ -104,6 +119,7 @@ void test_invalid_directive (void)
   T ("abc%Q");    /* { dg-warning "unknown conversion type character .Q." } */
 }
 
+
 /* Use 'grep "^ *T (" builtin-sprintf-6.c  | wc -l' to determine
    the count for the directive below.
-   { dg-final { scan-tree-dump-times "snprintf" 42 "optimized"} } */
+   { dg-final { scan-tree-dump-times "snprintf" 46 "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
index fae584e..76828cb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -429,11 +429,11 @@ void test_sprintf_chk_hh_const (void)
   T (4, "%hhi %hhi", 11, 12);   /* { dg-warning "into a region" } */
 
   /*  As a special case, a precision of zero with an argument of zero
-      results in zero bytes (unless modified by width).  */
+      results in zero bytes (unless modified by flags and/or width).  */
   T (1, "%.0hhd",   0);
-  T (1, "%+.0hhd",  0);
+  T (1, "%+.0hhd",  0);         /* { dg-warning "nul past the end" } */
   T (1, "%-.0hhd",  0);
-  T (1, "% .0hhd",  0);
+  T (1, "% .0hhd",  0);         /* { dg-warning "nul past the end" } */
   T (1, "%0.0hhd",  0);         /* { dg-warning ".0. flag ignored with precision" } */
   T (1, "%00.0hhd", 0);         /* { dg-warning "repeated .0. flag in format" } */
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
@@ -441,7 +441,8 @@ void test_sprintf_chk_hh_const (void)
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
   T (1, "%.0hhi",   0);
   T (1, "%.0hho",   0);
-  T (1, "%#.0hho",  0);
+  /* As a special case, '#' in base 8 results in 1 byte (the zero).  */
+  T (1, "%#.0hho",  0);         /* { dg-warning "nul past the end" } */
   T (1, "%.0hhx",   0);
   T (1, "%.0hhX",   0);
   T (1, "%#.0hhX",  0);
@@ -545,9 +546,9 @@ void test_sprintf_chk_h_const (void)
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
   T (1, "%.0hd",        0);
-  T (1, "%+.0hd",       0);
+  T (1, "%+.0hd",       0);         /* { dg-warning "nul past the end" } */
   T (1, "%-.0hd",       0);
-  T (1, "% .0hd",       0);
+  T (1, "% .0hd",       0);         /* { dg-warning "nul past the end" } */
   T (1, "%0.0hd",       0);         /* { dg-warning ".0. flag ignored with precision" } */
   T (1, "%00.0hd",      0);         /* { dg-warning "repeated .0. flag in format" } */
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
@@ -555,7 +556,7 @@ void test_sprintf_chk_h_const (void)
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
   T (1, "%.0hi",        0);
   T (1, "%.0ho",        0);
-  T (1, "%#.0ho",       0);
+  T (1, "%#.0ho",       0);         /* { dg-warning "nul past the end" } */
   T (1, "%.0hx",        0);
   T (1, "%.0hX",        0);
   T (1, "%#.0hX",       0);
@@ -628,9 +629,9 @@ void test_sprintf_chk_integer_const (void)
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
   T (1, "%.0d",         0);
-  T (1, "%+.0d",        0);
+  T (1, "%+.0d",        0);         /* { dg-warning "nul past the end" } */
   T (1, "%-.0d",        0);
-  T (1, "% .0d",        0);
+  T (1, "% .0d",        0);         /* { dg-warning "nul past the end" } */
   T (1, "%0.0d",        0);         /* { dg-warning ".0. flag ignored with precision" } */
   T (1, "%00.0d",       0);         /* { dg-warning "repeated .0. flag in format" } */
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
@@ -638,7 +639,7 @@ void test_sprintf_chk_integer_const (void)
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
   T (1, "%.0i",         0);
   T (1, "%.0o",         0);
-  T (1, "%#.0o",        0);
+  T (1, "%#.0o",        0);         /* { dg-warning "nul past the end" } */
   T (1, "%.0x",         0);
   T (1, "%.0X",         0);
   T (1, "%#.0X",        0);
@@ -727,9 +728,11 @@ void test_sprintf_chk_j_const (void)
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
   T (1, "%.0jd",     I (0));
-  T (1, "%+.0jd",    I (0));
+  T (1, "%+.0jd",    I (0));         /* { dg-warning "nul past the end" } */
+  T (1, "%+.0ju",    I (0));         /* { dg-warning ".\\+. flag used" } */
   T (1, "%-.0jd",    I (0));
-  T (1, "% .0jd",    I (0));
+  T (1, "% .0jd",    I (0));         /* { dg-warning "nul past the end" } */
+  T (1, "% .0ju",    I (0));         /* { dg-warning ". . flag used" } */
   T (1, "%0.0jd",    I (0));         /* { dg-warning ".0. flag ignored with precision" } */
   T (1, "%00.0jd",   I (0));         /* { dg-warning "repeated .0. flag in format" } */
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
@@ -737,7 +740,7 @@ void test_sprintf_chk_j_const (void)
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
   T (1, "%.0ji",     I (0));
   T (1, "%.0jo",     I (0));
-  T (1, "%#.0jo",    I (0));
+  T (1, "%#.0jo",    I (0));         /* { dg-warning "nul past the end" } */
   T (1, "%.0jx",     I (0));
   T (1, "%.0jX",     I (0));
   T (1, "%#.0jX",    I (0));
@@ -801,9 +804,11 @@ void test_sprintf_chk_l_const (void)
   /*  As a special case, a precision of zero with an argument of zero
       results in zero bytes (unless modified by width).  */
   T (1, "%.0ld",     0L);
-  T (1, "%+.0ld",    0L);
+  T (1, "%+.0ld",    0L);         /* { dg-warning "nul past the end" } */
+  T (1, "%+.0lu",    0L);         /* { dg-warning ".\\+. flag used" } */
   T (1, "%-.0ld",    0L);
-  T (1, "% .0ld",    0L);
+  T (1, "% .0ld",    0L);         /* { dg-warning "nul past the end" } */
+  T (1, "% .0lu",    0L);         /* { dg-warning ". . flag used" } */
   T (1, "%0.0ld",    0L);         /* { dg-warning ".0. flag ignored with precision" } */
   T (1, "%00.0ld",   0L);         /* { dg-warning "repeated .0. flag in format" } */
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
@@ -811,7 +816,7 @@ void test_sprintf_chk_l_const (void)
   /* { dg-warning ".0. flag ignored with precision" "" { target *-*-* } .-1 } */
   T (1, "%.0li",     0L);
   T (1, "%.0lo",     0L);
-  T (1, "%#.0lo",    0L);
+  T (1, "%#.0lo",    0L);         /* { dg-warning "nul past the end" } */
   T (1, "%.0lx",     0L);
   T (1, "%.0lX",     0L);
   T (1, "%#.0lX",    0L);
@@ -1039,8 +1044,8 @@ void test_sprintf_chk_s_nonconst (int w, int p, const char *s)
      is not.  */
   T ( 1, "%*s",  w, "");
   T ( 1, "%*s",  w, "1");       /* { dg-warning "nul past the end" } */
-  T ( 1, "%.*s", w, "");
-  T ( 1, "%.*s", w, "1");       /* { dg-warning "may write a terminating nul" } */
+  T ( 1, "%.*s", p, "");
+  T ( 1, "%.*s", p, "1");       /* { dg-warning "may write a terminating nul" } */
   T ( 1, "%.*s", w, "123");     /* { dg-warning "writing between 0 and 3 bytes into a region of size 1" } */
 
   T ( 1, "%*s", w, "123");      /* { dg-warning "writing 3 or more bytes into a region of size 1" } */
@@ -1294,6 +1299,12 @@ void test_sprintf_chk_int_nonconst (int w, int p, int a)
   T (3, "%2x",          a);
 
   T (1, "%.*d",      p, a);
+
+  T (4, "%i %i",        a, a);
+  /* The following will definitely be "writing a terminating nul past the end"
+     (i.e., not "may write".)  */
+  T (4, "%i %i ",       a, a);      /* { dg-warning "writing a terminating nul past the end" } */
+  T (4, "%i %i %i",     a, a, a);   /* { dg-warning "into a region" }*/
 }
 
 void test_sprintf_chk_e_nonconst (int w, int p, double d)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-2.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-2.c
index 3b57c0e..7acb83d 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-2.c
@@ -125,12 +125,61 @@ void test_s_nonconst (const char *s, const wchar_t *ws, struct Arrays *a)
 
   /* Exercise buffer overflow detection with non-const integer arguments.  */
 
-void test_hh_nonconst (int x)
+void test_hh_nonconst (int w, int p, int x, unsigned y)
 {
   T (1, "%hhi",         x);     /* { dg-warning "into a region" } */
   T (2, "%hhi",         x);     /* { dg-warning "into a region" } */
   T (3, "%hhi",         x);     /* { dg-warning "into a region" } */
   T (4, "%hhi",         x);     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T (1, "%hhi",         y);     /* { dg-warning "between 1 and 4 bytes" } */
+  T (2, "%hhi",         y);     /* { dg-warning "into a region" } */
+  T (3, "%hhi",         y);     /* { dg-warning "into a region" } */
+  T (4, "%hhi",         y);     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  /* Negative precision is treated as if none were specified.  */
+  T (1, "%.*hhi",   -1, x);     /* { dg-warning "between 1 and 4 bytes" } */
+  T (2, "%.*hhi",   -1, x);     /* { dg-warning "into a region" } */
+  T (3, "%.*hhi",   -1, x);     /* { dg-warning "into a region" } */
+  T (4, "%.*hhi",   -1, x);     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  /* Zero precision means that zero argument formats as no bytes unless
+     length or flags make it otherwise.  */
+  T (1, "%.*hhi",    0, x);     /* { dg-warning "between 0 and 4 bytes" } */
+  T (2, "%.*hhi",    0, x);     /* { dg-warning "between 0 and 4 bytes" } */
+  T (3, "%.*hhi",    0, x);     /* { dg-warning "between 0 and 4 bytes" } */
+  T (4, "%.*hhi",    0, x);     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T (1, "%.*hhi",    0, y);     /* { dg-warning "between 0 and 4 bytes" } */
+  T (2, "%.*hhi",    0, y);     /* { dg-warning "between 0 and 4 bytes" } */
+  T (3, "%.*hhi",    0, y);     /* { dg-warning "between 0 and 4 bytes" } */
+  T (4, "%.*hhi",    0, y);     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T (1, "%#.*hhi",    0, y);    /* { dg-warning "between 0 and 4 bytes" } */
+  /* { dg-warning ".#. flag used" "-Wformat" { target *-*-* } .-1 } */
+  T (1, "%+.*hhi",    0, y);    /* { dg-warning "between 1 and 4 bytes" } */
+  T (1, "%-.*hhi",    0, y);    /* { dg-warning "between 0 and 4 bytes" } */
+  T (1, "% .*hhi",    0, y);    /* { dg-warning "between 1 and 4 bytes" } */
+
+  T (1, "%#.*hhi",    1, y);    /* { dg-warning "between 1 and 4 bytes" } */
+  /* { dg-warning ".#. flag used" "-Wformat" { target *-*-* } .-1 } */
+  T (1, "%+.*hhi",    1, y);    /* { dg-warning "between 2 and 4 bytes" } */
+  T (1, "%-.*hhi",    1, y);    /* { dg-warning "between 1 and 4 bytes" } */
+  T (1, "% .*hhi",    1, y);    /* { dg-warning "between 2 and 4 bytes" } */
+
+  T (1, "%#.*hhi",    p, y);    /* { dg-warning "writing 0 or more bytes" } */
+  /* { dg-warning ".#. flag used" "-Wformat" { target *-*-* } .-1 } */
+  T (1, "%+.*hhi",    p, y);    /* { dg-warning "writing 1 or more bytes" } */
+  T (1, "%-.*hhi",    p, y);    /* { dg-warning "writing 0 or more bytes" } */
+  T (1, "% .*hhi",    p, y);    /* { dg-warning "writing 1 or more bytes" } */
+
+  T (1, "%#.*hhu",    0, y);    /* { dg-warning "between 0 and 3 bytes" } */
+  /* { dg-warning ".#. flag used" "-Wformat" { target *-*-* } .-1 } */
+  T (1, "%+.*hhu",    0, y);    /* { dg-warning "between 0 and 3 bytes" } */
+  /* { dg-warning ".\\+. flag used" "-Wformat" { target *-*-* } .-1 } */
+  T (1, "%-.*hhu",    0, y);    /* { dg-warning "between 0 and 3 bytes" } */
+  T (1, "% .*hhu",    0, y);    /* { dg-warning "between 0 and 3 bytes" } */
+  /* { dg-warning ". . flag used" "-Wformat" { target *-*-* } .-1 } */
 }
 
 void test_h_nonconst (int x)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
index 00176ed..21d27bf 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
@@ -116,54 +116,76 @@ void test_sprintf_chk_integer_value (void)
   T ( 9, "%8u", i (    1));
 }
 
+extern int rand (void);
+
 /* Functions to require optimization to figure out the range of the operand.
    Used to verify that the checker makes use of the range information to
    avoid diagnosing the output of sufficiently constrained arguments to
    integer directives.  */
 
-signed char*
-range_schar (signed char *val, signed char min, signed char max)
+signed char
+range_schar (signed char min, signed char max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  signed char val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-unsigned char*
-range_uchar (unsigned char *val, unsigned char min, unsigned char max)
+unsigned char
+range_uchar (unsigned char min, unsigned char max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  unsigned char val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-signed short*
-range_sshort (signed short *val, signed short min, signed short max)
+signed short
+range_sshrt (signed short min, signed short max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  signed short val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-unsigned short*
-range_ushort (unsigned short *val, unsigned short min, unsigned short max)
+unsigned short
+range_ushrt (unsigned short min, unsigned short max)
 {
-  if (*val < min || max < *val) __builtin_abort ();
-  return val;
+  unsigned short val = rand ();
+  return val < min || max < val ? min : val;
 }
 
-/* Helper to prevent GCC from figuring out the return value.  */
-extern int idx (void);
+signed int
+range_sint (signed int min, signed int max)
+{
+  signed int val = rand ();
+  return val < min || max < val ? min : val;
+}
 
-/* Exercise ranges only in types signed and unsigned char and short.
-   No other types work due to bug 71690.  */
+unsigned int
+range_uint (unsigned int min, unsigned int max)
+{
+  unsigned int val = rand ();
+  return val < min || max < val ? min : val;
+}
 
-void test_sprintf_chk_range_schar (signed char *a)
+void test_sprintf_chk_range_schar (void)
 {
-  (void)&a;
+#define R(min, max) range_sint (min, max)
+
+  T ( 0, "%hhi", R (0, 1));     /* { dg-warning ".%hhi. directive writing 1 byte into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[0, 1\\\]" "note" { target *-*-* } .-1 } */
 
-  /* Ra creates a range of signed char for A [idx].  A different
-     value is used each time to prevent the ranges from intesecting
-     one another, possibly even eliminating some tests as a result
-     of the range being empty.  */
-#define R(min, max) *range_schar (a + idx (), min, max)
+  T ( 0, "%hhi", R (0, 127));   /* { dg-warning ".%hhi. directive writing between 1 and 3 bytes into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[0, 127\\\]" "note" { target *-*-* } .-1 } */
+
+  T ( 0, "%hhi", R (1024, 1033));   /* { dg-warning ".%hhi. directive writing 1 byte into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[1024, 1033\\\]" "note" { target *-*-* } .-1 } */
+
+  T ( 0, "%hhi", R (1024, 1034));   /* { dg-warning ".%hhi. directive writing between 1 and 2 bytes into a region of size 0" } */
+  /* { dg-message "directive argument in the range \\\[1024, 1034\\\]" "note" { target *-*-* } .-1 } */
+
+  T ( 0, "%hhi", R (1024, 2035));   /* { dg-warning ".%hhi. directive writing between 1 and 4 bytes into a region of size 0" } */
+  /* { dg-message "using the range \\\[0, -128\\\] for directive argument" "note" { target *-*-* } .-1 } */
+
+#undef R
+#define R(min, max) range_schar (min, max)
 
   T ( 0, "%i",  R (0, 9));      /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
   T ( 1, "%i",  R (0, 9));      /* { dg-warning "nul past the end" } */
@@ -192,47 +214,108 @@ void test_sprintf_chk_range_schar (signed char *a)
   T ( 6, "%i_%i_%i", R (0, 9), R (0, 10), R (0, 10)); /* { dg-warning "may write a terminating nul past the end|.%i. directive writing between 1 and 2 bytes into a region of size 1" } */
 }
 
-void test_sprintf_chk_range_uchar (unsigned char *a, unsigned char *b)
+void test_sprintf_chk_range_uchar (void)
+{
+#undef R
+#define R(min, max) range_uchar (min, max)
+
+  T ( 0, "%i",  R (0,  9));   /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R (0,  9));   /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R (0,  9));
+  T ( 2, "%i",  R (9, 10));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R (0,  99));
+  T ( 3, "%i",  R (0, 100));  /* { dg-warning "may write a terminating nul past the end of the destination" } */
+}
+
+void test_sprintf_chk_range_sshrt (void)
+{
+#undef R
+#define R(min, max) range_sshrt (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R (-1, 0));     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
+}
+
+void test_sprintf_chk_range_ushrt (void)
 {
-  (void)&a;
-  (void)&b;
+#undef R
+#define R(min, max) range_ushrt (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
+}
 
-#undef Ra
-#define Ra(min, max) *range_uchar (a + idx (), min, max)
+void test_sprintf_chk_range_sint (void)
+{
+#undef R
+#define R(min, max) range_sint (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R (-1, 0));     /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
 
-  T ( 0, "%i",  Ra (0,  9));   /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
-  T ( 1, "%i",  Ra (0,  9));   /* { dg-warning "nul past the end" } */
-  T ( 2, "%i",  Ra (0,  9));
-  T ( 2, "%i",  Ra (9, 10));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
 
-  T ( 3, "%i",  Ra (0,  99));
-  T ( 3, "%i",  Ra (0, 100));  /* { dg-warning "may write a terminating nul past the end of the destination" } */
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
 }
 
-void test_sprintf_chk_range_sshort (signed short *a, signed short *b)
+void test_sprintf_chk_range_uint (void)
 {
-  (void)&a;
-  (void)&b;
-
-#undef Ra
-#define Ra(min, max) *range_sshort (a + idx (), min, max)
-
-  T ( 0, "%i",  Ra ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
-  T ( 1, "%i",  Ra ( 0, 1));     /* { dg-warning "nul past the end" } */
-  T ( 1, "%i",  Ra ( 0, 9));     /* { dg-warning "nul past the end" } */
-  T ( 2, "%i",  Ra ( 0, 1));
-  T ( 2, "%i",  Ra ( 8, 9));
-  T ( 2, "%i",  Ra ( 0, 9));
-  T ( 2, "%i",  Ra (-1, 0));     /* { dg-warning "may write a terminating nul past the end of the destination" } */
-  T ( 2, "%i",  Ra ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
-
-  T ( 3, "%i",  Ra ( 0, 99));
-  T ( 3, "%i",  Ra (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
-
-  T ( 4, "%i",  Ra (  0,  999));
-  T ( 4, "%i",  Ra ( 99,  999));
-  T ( 4, "%i",  Ra (998,  999));
-  T ( 4, "%i",  Ra (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
+#undef R
+#define R(min, max) range_uint (min, max)
+
+  T ( 0, "%i",  R ( 0, 9));     /* { dg-warning ".%i. directive writing 1 byte into a region of size 0" } */
+  T ( 1, "%i",  R ( 0, 1));     /* { dg-warning "nul past the end" } */
+  T ( 1, "%i",  R ( 0, 9));     /* { dg-warning "nul past the end" } */
+  T ( 2, "%i",  R ( 0, 1));
+  T ( 2, "%i",  R ( 8, 9));
+  T ( 2, "%i",  R ( 0, 9));
+  T ( 2, "%i",  R ( 9, 10));    /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 3, "%i",  R ( 0, 99));
+  T ( 3, "%i",  R (99, 999));   /* { dg-warning "may write a terminating nul past the end of the destination" } */
+
+  T ( 4, "%i",  R (  0,  999));
+  T ( 4, "%i",  R ( 99,  999));
+  T ( 4, "%i",  R (998,  999));
+  T ( 4, "%i",  R (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
 }
 
 /* Verify that destination size in excess of INT_MAX (and, separately,
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
index 0cb02b7..121ed4e 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c
@@ -2,15 +2,18 @@
    Test to verify that the correct range information is made available to the
    -Wformat-lenght check to prevent warnings.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wformat -Wformat-length" } */
+/* { dg-options "-O2 -Wformat -Wformat-length -fdump-tree-optimized" } */
 
+void abort (void);
 int snprintf (char*, __SIZE_TYPE__, const char*, ...);
 
 void fuchar (unsigned char j, char *p)
 {
   if (j > 99)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fschar (signed char j, char *p)
@@ -20,14 +23,17 @@ void fschar (signed char j, char *p)
   if (k > 99)
     return;
 
-  snprintf (p, 3, "%3hhu", k);   /* { dg-bogus "" "unsigned char" { xfail *-*-* } } */
+  if (3 != snprintf (p, 4, "%3hhu", k))
+    abort ();
 }
 
 void fushrt (unsigned short j, char *p)
 {
   if (j > 999)
     return;
-  snprintf (p, 4, "%3hu", j);
+
+  if (3 != snprintf (p, 4, "%3hu", j))
+    abort ();
 }
 
 void fshrt (short j, char *p)
@@ -37,7 +43,8 @@ void fshrt (short j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3hu", k);
+  if (3 != snprintf (p, 4, "%3hu", k))
+    abort ();
 }
 
 void fuint (unsigned j, char *p)
@@ -54,13 +61,16 @@ void fint (int j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3u", k);   /* { dg-bogus "" "unsigned int" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3u", k);
 }
 
 void fulong (unsigned long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3lu", j);
 }
 
@@ -71,13 +81,16 @@ void flong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3lu", k);   /* { dg-bogus "" "unsigned long" { xfail *-*-* } } */
+  /* Range info isn't available here.  */
+  snprintf (p, 4, "%3lu", k);
 }
 
 void fullong (unsigned long long j, char *p)
 {
   if (j > 999)
     return;
+
+  /* Range info isn't available here.  */
   snprintf (p, 4, "%3llu", j);
 }
 
@@ -88,5 +101,7 @@ void fllong (long j, char *p)
   if (k > 999)
     return;
 
-  snprintf (p, 4, "%3llu", k);   /* { dg-bogus "" "unsigned long long" { xfail lp64 } } */
+  snprintf (p, 4, "%3llu", k);
 }
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
index 916df79..35a5bd0 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
@@ -166,27 +166,48 @@ test_c (char c)
   EQL (5,  6, "%c %c %c",  c,   c,   c);
 }
 
-/* Generate a pseudo-random value in the specified range.  The return
-   value must be unsigned char to work around limitations in the GCC
-   range information.  Similarly for the declaration of rand() whose
-   correct return value should be int, but that also prevents the range
-   information from making it to the printf pass.  */
+/* Generate a pseudo-random unsigned value.  */
 
-unsigned char uchar_range (unsigned min, unsigned max)
+unsigned __attribute__ ((noclone, noinline))
+unsigned_value (void)
 {
-  extern unsigned rand (void);
+  extern int rand ();
+  return rand ();
+}
 
-  unsigned x;
-  x = rand ();
+/* Generate a pseudo-random signed value.  */
 
-  if (x < min)
-    x = min;
-  else if (max < x)
-    x = max;
+int __attribute__ ((noclone, noinline))
+int_value (void)
+{
+  extern int rand ();
+  return rand ();
+}
+
+/* Generate an unsigned char value in the specified range.  */
 
+static unsigned char
+uchar_range (unsigned min, unsigned max)
+{
+  unsigned x = unsigned_value ();
+  if (x < min || max < x)
+    x = min;
   return x;
 }
 
+/* Generate a signed int value in the specified range.  */
+
+static int
+int_range (int min, int max)
+{
+  int val = int_value ();
+  if (val < min || max < val)
+    val = min;
+  return val;
+}
+
+#define IR(min, max) int_range (min, max)
+
 static void __attribute__ ((noinline, noclone))
 test_d_i (int i, long li)
 {
@@ -266,9 +287,35 @@ test_d_i (int i, long li)
   RNG ( 1,  4,  5, "%hhi",     i);
   RNG ( 1,  3,  4, "%hhu",     i);
 
+  RNG ( 3,  4,  5, "%hhi",     IR (-128,  -10));
+  RNG ( 2,  4,  5, "%hhi",     IR (-128,   -1));
+  RNG ( 1,  4,  5, "%hhi",     IR (-128,    0));
+
+  RNG ( 1,  4,  5, "%1hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%2hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%3hhi",    IR (-128,    0));
+  RNG ( 1,  4,  5, "%4hhi",    IR (-128,    0));
+  RNG ( 1,  5,  6, "%5hhi",    IR (-128,    0));
+  RNG ( 1,  6,  7, "%6hhi",    IR (-128,    0));
+  RNG ( 2,  6,  7, "%6hhi",    IR (-128,   10));
+
+  RNG ( 0,  1,  2, "%.hhi",    IR (   0,    1));
+  RNG ( 0,  1,  2, "%.0hhi",   IR (   0,    1));
+  RNG ( 0,  1,  2, "%0.0hhi",  IR (   0,    1));   /* { dg-warning ".0. flag ignored with precision" } */
+  RNG ( 0,  1,  2, "%*.0hhi",  0, IR (   0,    1));
+
+  RNG ( 1,  2,  3, "%hhi",     IR (1024, 1034));
+  RNG ( 1,  4,  5, "%hhi",     IR (1024, 2048));
+  RNG ( 2,  3,  4, "%hhi",     IR (1034, 1151));
+
+  RNG ( 1,  2,  3, "%hhu",     IR (1024, 1034));
+  RNG ( 1,  3,  4, "%hhu",     IR (1024, 2048));
+  RNG ( 2,  3,  4, "%hhu",     IR (1034, 1151));
+
 #if __SIZEOF_SHORT__ == 2
   RNG ( 1,  6,  7, "%hi",      i);
   RNG ( 1,  5,  6, "%hu",      i);
+
 #elif __SIZEOF_SHORT__ == 4
   RNG ( 1, 11, 12, "%hi",      i);
   RNG ( 1, 10, 11, "%hu",      i);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78622.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78622.c
new file mode 100644
index 0000000..7a73ebe
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78622.c
@@ -0,0 +1,63 @@
+/* PR middle-end/78622 - [7 Regression] -Wformat-length/-fprintf-return-value
+   incorrect with overflow/wrapping
+   { dg-do compile }
+   { dg-options "-Wformat-length=2" } */
+
+char buf[1];
+
+int test_uchar_hhd (unsigned char x)
+{
+  if (x < 64 || x > 2U * __SCHAR_MAX__ - 10)
+    return -1;
+
+  return __builtin_sprintf (buf, "%hhd", x + 1);   /* { dg-warning "directive writing between 1 and 4 bytes into a region of size 1" "int32plus" { target { int32plus } } } */
+}
+
+int test_uint_hhd (unsigned x)
+{
+  if (x < 64 || x > 2U * __INT_MAX__ - 10)
+    return -1;
+
+  return __builtin_sprintf (buf, "%hhd", x + 1);   /* { dg-warning "directive writing between 1 and 4 bytes into a region of size 1" "int32plus" { target { int32plus } } } */
+}
+
+int test_schar_hhu (signed char x)
+{
+  if (x < -9 || x > 9)
+    return -1;
+
+  return __builtin_sprintf (buf, "%hhu", x + 1);   /* { dg-warning "directive writing between 1 and 3 bytes into a region of size 1" "int32plus" { target { int32plus } } } */
+}
+
+int test_ushort_hd (unsigned short x)
+{
+  if (x < 64 || x > 2U * __SHRT_MAX__ - 10)
+    return -1;
+
+  return __builtin_sprintf (buf, "%hd", x + 1);   /* { dg-warning "directive writing between 1 and 6 bytes into a region of size 1" "int32plus" { target { int32plus } } } */
+}
+
+int test_uint_d (unsigned x)
+{
+  if (x < 64 || x > 2U * __INT_MAX__ - 10)
+    return -1;
+
+  return __builtin_sprintf (buf, "%d", x + 1);   /* { dg-warning "directive writing between 1 and 11 bytes into a region of size 1" "int32plus" { target { int32plus } } } */
+}
+
+int test_ulong_ld (unsigned long x)
+{
+  if (x < 64 || x > 2LU * __LONG_MAX__ - 10)
+    return -1;
+
+  return __builtin_sprintf (buf, "%ld", x + 1);   /* { dg-warning "directive writing between 1 and 11 bytes into a region of size 1" "ilp32" { target { ilp32 } } } */
+  /* { dg-warning "directive writing between 1 and 20 bytes into a region of size 1" "lp64" { target { lp64 } } .-1 } */
+}
+
+int test_ullong_lld (unsigned long long x)
+{
+  if (x < 64 || x > 2LLU * __LONG_LONG_MAX__ - 10)
+    return -1;
+
+  return __builtin_sprintf (buf, "%lld", x + 1);   /* { dg-warning "directive writing between 1 and 20 bytes into a region of size 1" "int32plus" { target { int32plus } } } */
+}

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jeff Law]

On 12/07/2016 12:17 PM, Martin Sebor wrote:
>> OK.  So is the hangup really a problem in how the return type is
>> documented?  I parsed the comment as essentially saying we return true
>> if the range gets adjusted in any way -- thus a sign change in the first
>> block would qualify, but we returned false which seemed inconsistent.
>>
>> Looking at it again, what I think it's saying is we're returning true
>> only for a subset of adjustments to the range, ie, those cases where the
>> postcondition does not hold.  Correct?
>
> Correct.  Would changing the description of the return value to
> this make it clearer?
>
>    Return true when the range has been adjusted to the full unsigned
>    range of DIRTYPE, [0, DIRTYPE_MAX], false otherwise.
Yea, that does help.  I see you've got an updated version posted.  Let 
me take a final (?) looksie now that I have a better understanding of 
the return value.  I think that was the biggest stumbling block on my side.

Jeff

Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)

[Jeff Law]

On 12/11/2016 05:21 PM, Martin Sebor wrote:
>> So I think the return value needs a bit of clarification here.  Take
>> guidance from our discussion on this thread.
>>
>> OK with that fixed.
>>
>> jeff
>
> The "strange test failures​" that I wrote about in a separate thread
> late last week prompted me to re-review the patch and add more test
> cases.  Those in turn exposed a bug in the adjust_range_for_overflow
> function involving types of the same precision but different sign
> where converting an unsigned range with an upper bound in excess of
> the directive's TYPE_MAX would incorrectly accept the converted range
> even though the new upper bound was less than the lower bound.
>
> The updated  patch corrects this oversight.  In addition, it adjusts
> the handling of the obscure corner case of zero precision and zero
> argument which results in zero bytes (except in some even more
> obscure cases involving some flags for some conversions).  For
> instance:
>
>   printf ("%.0i", 0);
>
> results in zero bytes, but
>
>   printf ("%+.0i", 0);
>
> results in 1 byte (and prints '+').  This is tracked in bug 78606.
>
> Although the differences between the approved patch and the update
> are very small I repost it in case one of you would like to double
> check them.  If not I'll commit the updated patch later in the week.
>
> Martin
>
> gcc-78622.diff
>
>
> PR middle-end/78622 - -Wformat-length/-fprintf-return-value incorrect with overflow/wrapping
> PR middle-end78606 - -Wformat-length/-fprintf-return-value incorrect for %+.0i and %.0o with zero value
>
> gcc/ChangeLog:
>
> 	PR middle-end/78622
> 	* gimple-ssa-sprintf.c (min_bytes_remaining): Use res.knownrange
> 	rather than res.bounded.
> 	(get_width_and_precision): Set precision to -1 when negative.
> 	(adjust_range_for_overflow): New function.
> 	(format_integer): Correct the handling of the space, plus, and pound
> 	flags, and the special case of zero precision.
> 	Always set res.bounded to true unless either precision or width
> 	is specified and unknown.
> 	Call adjust_range_for_overflow.
> 	Avoid use zero as the shortest value when precision is specified
> 	but unknown.
> 	(format_directive): Remove vestigial quoting.  Always inform of
> 	argument value or range when it's available.
> 	(add_bytes): Correct the computation of boundrange used to
> 	decide whether a warning is of a "maybe" or "defnitely" kind.
>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/78622
> 	* gcc.c-torture/execute/pr78622.c: New test.
> 	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Remove "benign" undefined
> 	behavior inadvertently introduced in a previous commit.  Tighten
> 	up final checking.
> 	* gcc.dg/tree-ssa/builtin-sprintf-5.c: Rename macros for clarity.
> 	Add test cases.
> 	* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-2.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: Same.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: Remove xfails and
> 	add a final optimization check.
> 	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.
> 	* gcc.dg/tree-ssa/pr78622.c: New test.
>
OK.

jeff