Subject: bug#18857: closed (Re: [bug-diffutils] bug#18857: floating point exception on invalid argument)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 28 Oct 2014 05:26:03 +0000

Your bug report

#18857: floating point exception on invalid argument

which was filed against the diffutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
18857: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18857
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [bug-diffutils] bug#18857: floating point exception on invalid argument
From: Paul Eggert <[email-masked]>
To: Tobias Stoeckmann <[email-masked]>, [email-masked]
Date: Mon, 27 Oct 2014 22:24:48 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Tobias Stoeckmann wrote:
> I don't see the need to adjust anything
> except avoiding this special value.  I doubt that anyone ever needed
> these large numbers.

Yes, it's hard to imagine anyone needing a tabsize that large.  Still, the GNU 
policy is to avoid unnecessary limits, so it's better to make the limit as large 
as easily possible, which here would be SIZE_MAX - GUTTER_WIDTH_MINIMUM.  Also, 
I see there are other ways the nearby code can overflow.  Plus, we should put in 
a test case for this bug.  So I installed the attached patch, which should 
address these issues.  Thanks for reporting the problem.
From 952a704d6b81f5283ec2cede4b9995f04494e1ad Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 27 Oct 2014 19:53:08 -0700
Subject: [PATCH] diff: fix integer overflow problem with --tabsize

Reported by Tobias Stoeckmann in: http://bugs.gnu.org/18857
* src/diff.c (main): Don't overflow if INTMAX_MAX / 2 < tabsize.
* tests/bignum: New file, to test for this bug.
* tests/Makefile.am (TESTS): Add it.
---
 src/diff.c        | 15 ++++++++++-----
 tests/Makefile.am |  1 +
 tests/bignum      | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)
 create mode 100755 tests/bignum

diff --git a/src/diff.c b/src/diff.c
index 397815e..03f8e4f 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -595,7 +595,8 @@ main (int argc, char **argv)
 
 	case TABSIZE_OPTION:
 	  numval = strtoumax (optarg, &numend, 10);
-	  if (! (0 < numval && numval <= SIZE_MAX) || *numend)
+	  if (! (0 < numval && numval <= SIZE_MAX - GUTTER_WIDTH_MINIMUM)
+	      || *numend)
 	    try_help ("invalid tabsize '%s'", optarg);
 	  if (tabsize != numval)
 	    {
@@ -682,10 +683,14 @@ main (int argc, char **argv)
 		a half line plus a gutter is an integral number of tabs,
 		so that tabs in the right column line up.  */
 
-    intmax_t t = expand_tabs ? 1 : tabsize;
-    intmax_t w = width;
-    intmax_t off = (w + t + GUTTER_WIDTH_MINIMUM) / (2 * t)  *  t;
-    sdiff_half_width = MAX (0, MIN (off - GUTTER_WIDTH_MINIMUM, w - off)),
+    size_t t = expand_tabs ? 1 : tabsize;
+    size_t w = width;
+    size_t t_plus_g = t + GUTTER_WIDTH_MINIMUM;
+    size_t unaligned_off = (w >> 1) + (t_plus_g >> 1) + (w & t_plus_g & 1);
+    size_t off = unaligned_off - unaligned_off % t;
+    sdiff_half_width = (off <= GUTTER_WIDTH_MINIMUM || w <= off
+			? 0
+			: MIN (off - GUTTER_WIDTH_MINIMUM, w - off));
     sdiff_column2_offset = sdiff_half_width ? off : w;
   }
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 005d9f0..438fbdf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -2,6 +2,7 @@
 
 TESTS = \
   basic \
+  bignum \
   binary \
   colliding-file-names \
   excess-slash \
diff --git a/tests/bignum b/tests/bignum
new file mode 100755
index 0000000..a0c95f6
--- /dev/null
+++ b/tests/bignum
@@ -0,0 +1,14 @@
+#!/bin/sh
+# big numbers
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+fail=0
+
+for tabsize in 2147483648 9223372036854775808; do
+  diff --tabsize=$tabsize /dev/null /dev/null
+  status=$?
+  test $status -eq 0 || test $status -eq 2 || fail=1
+done
+
+Exit $fail
-- 
1.9.3

Subject: floating point exception on invalid argument
From: Tobias Stoeckmann <[email-masked]>
To: [email-masked]
Date: Mon, 27 Oct 2014 21:06:01 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

on 64 bit systems, an invalid tabsize argument can trigger a floating
point exception:

$ diff --tabsize=9223372036854775808 . .
Floating point exception

Offending line is this one:
    intmax_t off = (w + t + GUTTER_WIDTH_MINIMUM) / (2 * t)  *  t;

The tabsize I supplied is 2^63, multiplying by 2 will be 2^64 which
is too large, so the divisor overflows to 0.

This is a purely cosmetical fix, I don't see the need to adjust anything
except avoiding this special value.  I doubt that anyone ever needed
these large numbers.

Tobias

--- diffutils-3.3/src/diff.c~	2014-10-27 20:54:30.968656876 +0100
+++ diffutils-3.3/src/diff.c	2014-10-27 20:54:41.360708407 +0100
@@ -594,7 +594,7 @@
 
 	case TABSIZE_OPTION:
 	  numval = strtoumax (optarg, &numend, 10);
-	  if (! (0 < numval && numval <= SIZE_MAX) || *numend)
+	  if (! (0 < numval && numval <= SIZE_MAX / 2) || *numend)
 	    try_help ("invalid tabsize '%s'", optarg);
 	  if (tabsize != numval)
 	    {