[PATCH] numfmt: avoid integer overflow when rounding

[Pádraig Brady]

Due to existing limits this is usually triggered
with an increased precision.  We also add further
restrictions to the output of increased precision numbers.

* src/numfmt.c (simple_round): Avoid intmax_t overflow.
(simple_strtod_int): Count digits consistently
for precision loss and overflow detection.
(prepare_padded_number): Include the precision
when excluding numbers to output, since the precision
determines the ultimate values used in the rounding scheme
in double_to_human().
* tests/misc/numfmt.pl: Add previously failing test cases.
* NEWS: Mention the fix.
---
 NEWS                 |  4 ++++
 src/numfmt.c         | 51 ++++++++++++++++++++++++++++++++++++++-------------
 tests/misc/numfmt.pl | 11 +++++++++++
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 9b86d45..3b30000 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   even if the parent directory exists and has a different default context.
   [bug introduced with the -Z restorecon functionality in coreutils-8.22]
 
+  numfmt no longer outputs incorrect overflowed values seen with certain
+  large numbers, or with numbers with increased precision.
+  [bug introduced when numfmt was added in coreutils-8.21]
+
   paste no longer truncates output for large input files.  This would happen
   for example with files larger than 4GiB on 32 bit systems with a '\n'
   character at the 4GiB position.
diff --git a/src/numfmt.c b/src/numfmt.c
index 81c624a..82a9585 100644
--- a/src/numfmt.c
+++ b/src/numfmt.c
@@ -388,30 +388,41 @@ simple_round_nearest (long double val)
   return val < 0 ? val - 0.5 : val + 0.5;
 }
 
-static inline intmax_t
+static inline long double _GL_ATTRIBUTE_CONST
 simple_round (long double val, enum round_type t)
 {
+  intmax_t rval;
+  intmax_t intmax_mul = val / INTMAX_MAX;
+  val -= (long double) INTMAX_MAX * intmax_mul;
+
   switch (t)
     {
     case round_ceiling:
-      return simple_round_ceiling (val);
+      rval = simple_round_ceiling (val);
+      break;
 
     case round_floor:
-      return simple_round_floor (val);
+      rval = simple_round_floor (val);
+      break;
 
     case round_from_zero:
-      return simple_round_from_zero (val);
+      rval = simple_round_from_zero (val);
+      break;
 
     case round_to_zero:
-      return simple_round_to_zero (val);
+      rval = simple_round_to_zero (val);
+      break;
 
     case round_nearest:
-      return simple_round_nearest (val);
+      rval = simple_round_nearest (val);
+      break;
 
     default:
       /* to silence the compiler - this should never happen.  */
       return 0;
     }
+
+  return (long double) INTMAX_MAX * intmax_mul + rval;
 }
 
 enum simple_strtod_error
@@ -465,10 +476,11 @@ simple_strtod_int (const char *input_str,
     {
       int digit = (**endptr) - '0';
 
+      digits++;
+
       if (digits > MAX_UNSCALED_DIGITS)
         e = SSE_OK_PRECISION_LOSS;
 
-      ++digits;
       if (digits > MAX_ACCEPTABLE_DIGITS)
         return SSE_OVERFLOW;
 
@@ -519,7 +531,6 @@ simple_strtod_float (const char *input_str,
   if (e != SSE_OK && e != SSE_OK_PRECISION_LOSS)
     return e;
 
-
   /* optional decimal point + fraction.  */
   if (STREQ_LEN (*endptr, decimal_point, decimal_point_length))
     {
@@ -542,6 +553,8 @@ simple_strtod_float (const char *input_str,
 
       val_frac = ((long double) val_frac) / powerld (10, exponent);
 
+      /* TODO: detect loss of precision (only really 18 digits
+         of precision across all digits (before and after '.')).  */
       if (value)
         {
           if (negative)
@@ -1165,14 +1178,26 @@ prepare_padded_number (const long double val, size_t 
precision)
   /* Generate Output. */
   char buf[128];
 
+  size_t precision_used = user_precision == -1 ? precision : user_precision;
+
   /* Can't reliably print too-large values without auto-scaling. */
   unsigned int x;
   expld (val, 10, &x);
-  if (scale_to == scale_none && x > MAX_UNSCALED_DIGITS)
+
+  if (scale_to == scale_none
+      && x + precision_used > MAX_UNSCALED_DIGITS)
     {
       if (inval_style != inval_ignore)
-        error (conv_exit_code, 0, _("value too large to be printed: '%Lg'"
-                                    " (consider using --to)"), val);
+        {
+          if (precision_used)
+            error (conv_exit_code, 0,
+                   _("value/precision too large to be printed: 
'%Lg/%"PRIuMAX"'"
+                     " (consider using --to)"), val, 
(uintmax_t)precision_used);
+          else
+            error (conv_exit_code, 0,
+                   _("value too large to be printed: '%Lg'"
+                     " (consider using --to)"), val);
+        }
       return 0;
     }
 
@@ -1184,8 +1209,8 @@ prepare_padded_number (const long double val, size_t 
precision)
       return 0;
     }
 
-  double_to_human (val, user_precision == -1 ? precision : user_precision, buf,
-                   sizeof (buf), scale_to, grouping, round_style);
+  double_to_human (val, precision_used, buf, sizeof (buf),
+                   scale_to, grouping, round_style);
   if (suffix)
     strncat (buf, suffix, sizeof (buf) - strlen (buf) -1);
 
diff --git a/tests/misc/numfmt.pl b/tests/misc/numfmt.pl
index 4ed1d66..25bba61 100755
--- a/tests/misc/numfmt.pl
+++ b/tests/misc/numfmt.pl
@@ -21,6 +21,8 @@ use strict;
 (my $program_name = $0) =~ s|.*/||;
 my $prog = 'numfmt';
 
+my $limits = getlimits ();
+
 # TODO: add localization tests with "grouping"
 # Turn off localization of executable's output.
 @ENV{qw(LANGUAGE LANG LC_ALL)} = ('C') x 3;
@@ -534,6 +536,11 @@ my @Tests =
              {ERR => "$prog: value too large to be printed: '1e+19' " .
                      "(consider using --to)\n"},
              {EXIT=>2}],
+     ['large-4','1000000000000000000.0',
+             {ERR => "$prog: value/precision too large to be printed: " .
+                     "'1e+18/1' (consider using --to)\n"},
+             {EXIT=>2}],
+
 
      # Test input:
      # Up to 27 digits is OK.
@@ -648,6 +655,10 @@ my @Tests =
                      "(cannot handle values > 999Y)\n"},
              {EXIT => 2}],
 
+     # intmax_t overflow when rounding caused this to fail before 8.24
+     ['large-15',$limits->{INTMAX_OFLOW}, {OUT=>$limits->{INTMAX_OFLOW}}],
+     ['large-16','9.300000000000000000', {OUT=>'9.300000000000000000'}],
+
      # precision override
      ['precision-1','--format=%.4f 9991239123 --to=si', {OUT=>"9.9913G"}],
      ['precision-2','--format=%.1f 9991239123 --to=si', {OUT=>"10.0G"}],
-- 
2.4.1