diff mbox

correct off-by-one error in handling of precision of negative numbers (PR 78910)

Message ID 92847957-b488-9f2f-96b8-e443b9d2882a@gmail.com
State New
Headers show

Commit Message

Martin Sebor Jan. 4, 2017, 9:43 p.m. UTC
The attached patch corrects an off-by-one mistake in handling
non-zero precisions in signed conversions (%d and %i) with
negative arguments, such as in sprintf(d, "%.2", -1).  The call
which results in the three bytes "-01" on output, not 2.  I.e.,
the minus sign must be taken into consideration.

Thanks
Martin

Comments

Jeff Law Jan. 5, 2017, 6:19 p.m. UTC | #1
On 01/04/2017 02:43 PM, Martin Sebor wrote:
> The attached patch corrects an off-by-one mistake in handling

> non-zero precisions in signed conversions (%d and %i) with

> negative arguments, such as in sprintf(d, "%.2", -1).  The call

> which results in the three bytes "-01" on output, not 2.  I.e.,

> the minus sign must be taken into consideration.

>

> Thanks

> Martin

>

> gcc-78910.diff

>

>

> PR tree-optimization/78910 - Wrong print-return-value for a negative number

>

> gcc/ChangeLog:

>

> 	PR tree-optimization/78910

> 	* gimple-ssa-sprintf.c (tree_digits): Add an argument.

> 	(format_integer): Correct off-by-one error in the handling

> 	of precision with negative numbers in signed conversions..

>

> gcc/testsuite/ChangeLog:

>

> 	PR tree-optimization/78910

> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: Adjust text of expected

> 	diagnostics.

> 	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.

> 	* gcc.dg/tree-ssa/pr78910.c: New test.

OK.
jeff
diff mbox

Patch

PR tree-optimization/78910 - Wrong print-return-value for a negative number

gcc/ChangeLog:

	PR tree-optimization/78910
	* gimple-ssa-sprintf.c (tree_digits): Add an argument.
	(format_integer): Correct off-by-one error in the handling
	of precision with negative numbers in signed conversions..

gcc/testsuite/ChangeLog:

	PR tree-optimization/78910
	* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: Adjust text of expected
	diagnostics.
	* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.
	* gcc.dg/tree-ssa/pr78910.c: New test.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index d468cd7..6a9f679 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -549,17 +549,18 @@  ilog (unsigned HOST_WIDE_INT x, int base)
 }
 
 /* Return the number of bytes resulting from converting into a string
-   the INTEGER_CST tree node X in BASE.  PLUS indicates whether 1 for
-   a plus sign should be added for positive numbers, and PREFIX whether
-   the length of an octal ('O') or hexadecimal ('0x') prefix should be
-   added for nonzero numbers.  Return -1 if X cannot be represented.  */
-
-static int
-tree_digits (tree x, int base, bool plus, bool prefix)
+   the INTEGER_CST tree node X in BASE with a minimum of PREC digits.
+   PLUS indicates whether 1 for a plus sign should be added for positive
+   numbers, and PREFIX whether the length of an octal ('O') or hexadecimal
+   ('0x') prefix should be added for nonzero numbers.  Return -1 if X cannot
+   be represented.  */
+
+static HOST_WIDE_INT
+tree_digits (tree x, int base, HOST_WIDE_INT prec, bool plus, bool prefix)
 {
   unsigned HOST_WIDE_INT absval;
 
-  int res;
+  HOST_WIDE_INT res;
 
   if (TYPE_UNSIGNED (TREE_TYPE (x)))
     {
@@ -591,7 +592,9 @@  tree_digits (tree x, int base, bool plus, bool prefix)
 	return -1;
     }
 
-  res += ilog (absval, base);
+  int ndigs = ilog (absval, base);
+
+  res += prec < ndigs ? ndigs : prec;
 
   if (prefix && absval)
     {
@@ -1022,10 +1025,9 @@  format_integer (const conversion_spec &spec, tree arg)
 	  /* True when a conversion is preceded by a prefix indicating the base
 	     of the argument (octal or hexadecimal).  */
 	  bool maybebase = spec.get_flag ('#');
-	  len = tree_digits (arg, base, maybesign, maybebase);
-
-	  if (len < prec)
-	    len = prec;
+	  len = tree_digits (arg, base, prec, maybesign, maybebase);
+	  if (len < 1)
+	    len = HOST_WIDE_INT_MAX;
 	}
 
       if (len < width)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
index 7787255..e087a8f 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
@@ -35,11 +35,16 @@  void test_integer_var (int i)
   T (0, "%*d",  INT_MAX, i);     /* { dg-warning "writing 2147483647 bytes" } */
 
   T (0, "%.*d", INT_MIN, i);     /* { dg-warning "writing between 1 and 11 bytes" } */
-  T (0, "%.*d", INT_MAX, i);     /* { dg-warning "writing 2147483647 bytes" } */
+
+  /* The following writes INT_MAX digits and, when i is negative, a minus
+     sign.  */
+  T (0, "%.*d", INT_MAX, i);     /* { dg-warning "writing between 2147483647 and 2147483648 bytes" } */
 
   T (0, "%*.*d", INT_MIN, INT_MIN, i);   /* { dg-warning "writing 2147483648 bytes" } */
 
-  T (0, "%*.*d", INT_MAX, INT_MAX, i);   /* { dg-warning "writing 2147483647 bytes" } */
+  /* The following writes INT_MAX digits and, when i is negative, a minus
+     sign.  */
+  T (0, "%*.*d", INT_MAX, INT_MAX, i);   /* { dg-warning "writing between 2147483647 and 2147483648 bytes" } */
 }
 
 void test_floating_a_cst (void)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
index 96892c1..275cb28 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
@@ -319,17 +319,61 @@  test_d_i (int i, long li)
   RNG ( 1,  6,  7, "%hi",      i);
   RNG ( 1,  5,  6, "%hu",      i);
 
+  RNG ( 1,  6,  7, "%.1hi",    i);
+  RNG ( 2,  6,  7, "%.2hi",    i);
+  RNG ( 3,  6,  7, "%.3hi",    i);
+  RNG ( 4,  6,  7, "%.4hi",    i);
+  RNG ( 5,  6,  7, "%.5hi",    i);
+  RNG ( 6,  7,  8, "%.6hi",    i);
+  RNG ( 7,  8,  9, "%.7hi",    i);
+
 #elif __SIZEOF_SHORT__ == 4
   RNG ( 1, 11, 12, "%hi",      i);
   RNG ( 1, 10, 11, "%hu",      i);
+
+  RNG ( 1, 11, 12, "%.1hi",    i);
+  RNG ( 2, 11, 12, "%.2hi",    i);
+  RNG ( 3, 11, 12, "%.3hi",    i);
+  RNG ( 4, 11, 12, "%.4hi",    i);
+  RNG ( 5, 11, 12, "%.5hi",    i);
+  RNG ( 6, 11, 12, "%.6hi",    i);
+  RNG ( 7, 11, 12, "%.7hi",    i);
+  RNG ( 8, 11, 12, "%.8hi",    i);
+  RNG ( 9, 11, 12, "%.9hi",    i);
+  RNG (10, 11, 12, "%.10hi",   i);
+  RNG (11, 12, 13, "%.11hi",   i);
+  RNG (12, 13, 14, "%.12hi",   i);
+  RNG (13, 14, 15, "%.13hi",   i);
 #endif
 
 #if __SIZEOF_INT__ == 2
   RNG ( 1,  6,  7, "%i",       i);
   RNG ( 1,  5,  6, "%u",       i);
+
+  RNG ( 1,  6,  7, "%.1i",     i);
+  RNG ( 2,  6,  7, "%.2i",     i);
+  RNG ( 3,  6,  7, "%.3i",     i);
+  RNG ( 4,  6,  7, "%.4i",     i);
+  RNG ( 5,  6,  7, "%.5i",     i);
+  RNG ( 6,  7,  8, "%.6i",     i);
+  RNG ( 7,  8,  9, "%.7i",     i);
 #elif __SIZEOF_INT__ == 4
   RNG ( 1, 11, 12, "%i",       i);
   RNG ( 1, 10, 11, "%u",       i);
+
+  RNG ( 1, 11, 12, "%.1i",    i);
+  RNG ( 2, 11, 12, "%.2i",    i);
+  RNG ( 3, 11, 12, "%.3i",    i);
+  RNG ( 4, 11, 12, "%.4i",    i);
+  RNG ( 5, 11, 12, "%.5i",    i);
+  RNG ( 6, 11, 12, "%.6i",    i);
+  RNG ( 7, 11, 12, "%.7i",    i);
+  RNG ( 8, 11, 12, "%.8i",    i);
+  RNG ( 9, 11, 12, "%.9i",    i);
+  RNG (10, 11, 12, "%.10i",   i);
+  RNG (11, 12, 13, "%.11i",   i);
+  RNG (12, 13, 14, "%.12i",   i);
+  RNG (13, 14, 15, "%.13i",   i);
 #elif __SIZEOF_INT__ == 8
   RNG ( 1, 20, 21, "%i",       i);
   RNG ( 1, 19, 20, "%u",       i);
@@ -338,6 +382,20 @@  test_d_i (int i, long li)
 #if __SIZEOF_LONG__ == 4
   RNG ( 1, 11, 12, "%li",      li);
   RNG ( 1, 10, 11, "%lu",      li);
+
+  RNG ( 1, 11, 12, "%.1li",    li);
+  RNG ( 2, 11, 12, "%.2li",    li);
+  RNG ( 3, 11, 12, "%.3li",    li);
+  RNG ( 4, 11, 12, "%.4li",    li);
+  RNG ( 5, 11, 12, "%.5li",    li);
+  RNG ( 6, 11, 12, "%.6li",    li);
+  RNG ( 7, 11, 12, "%.7li",    li);
+  RNG ( 8, 11, 12, "%.8li",    li);
+  RNG ( 9, 11, 12, "%.9li",    li);
+  RNG (10, 11, 12, "%.10li",   li);
+  RNG (11, 12, 13, "%.11li",   li);
+  RNG (12, 13, 14, "%.12li",   li);
+  RNG (13, 14, 15, "%.13li",   li);
 #elif __SIZEOF_LONG__ == 8
   RNG ( 1, 20, 21, "%li",      li);
   RNG ( 1, 19, 20, "%lu",      li);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78910.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78910.c
new file mode 100644
index 0000000..ba5216e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78910.c
@@ -0,0 +1,15 @@ 
+/* PR tree-optimization/78910 - Wrong print-return-value for a negative number
+   { dg-do compile }
+   { dg-options "-O2 -fdump-tree-optimized" } */
+
+int main()
+{
+  char b[128];
+  int l = __builtin_sprintf (b, "%.2d", -1);
+  __builtin_printf ("b: '%s', length: %d\n", b, l);
+  if (l != 3)
+    __builtin_abort ();
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "abort" "optimized"} } */