On 14/06/2020 13:47, Tobias Stoeckmann wrote:
Since -LONG_MIN results in LONG_MIN again, the operation itself is
a signed integer overflow.
This can be observed with the following calls (best if compiled
with -ftrapv or -fsanitize=undefined):
$ numfmt --padding=-9223372036854775808
$ seq 1e-9223372036854775808
Technically, the change in seq "reduces" the precision, but a double
or long double that small would be represented as 0 anyway.
Thanks for fixing those -fsanitize=undefined issues.
I can confirm with GCC 10 -fsanitize=undefined was giving:
src/numfmt.c:1505:31: runtime error:
negation of -9223372036854775808 cannot be represented in type 'long int'
How did you notice BTW. This wasn't exposed in existing tests.
I've updated your patch (attached) to add tests for this.
cheers,
Pádraig
>From 0fe9fdece735234819ead2cad7902ba9a6cd8487 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <tob...@stoeckmann.org>
Date: Sun, 14 Jun 2020 14:47:11 +0200
Subject: [PATCH] maint: avoid signed integer overflows
Since -LONG_MIN results in LONG_MIN again, the operation itself is
a signed integer overflow.
This can be observed with the following calls (best if compiled
with -ftrapv or -fsanitize=undefined):
$ numfmt --padding=-9223372036854775808
$ seq 1e-9223372036854775808
Technically, the change in seq "reduces" the precision, but a double
or long double that small would be represented as 0 anyway.
* src/numfmt.c: Explicitly disallow --padding=LONG_MIN.
* src/seq.c: Treat 1e$LONG_MIN as 1e-$LONG_MAX.
* tests/misc/numfmt.pl: Add a test case.
* tests/misc/seq-precision.sh: Likewise.
Fixes https://bugs.gnu.org/41850
---
src/numfmt.c | 2 +-
src/seq.c | 2 +-
tests/misc/numfmt.pl | 3 +++
tests/misc/seq-precision.sh | 6 ++++++
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/numfmt.c b/src/numfmt.c
index 8869791b0..8871a8c01 100644
--- a/src/numfmt.c
+++ b/src/numfmt.c
@@ -1496,7 +1496,7 @@ main (int argc, char **argv)
case PADDING_OPTION:
if (xstrtol (optarg, NULL, 10, &padding_width, "") != LONGINT_OK
- || padding_width == 0)
+ || padding_width == 0 || padding_width < -LONG_MAX)
die (EXIT_FAILURE, 0, _("invalid padding value %s"),
quote (optarg));
if (padding_width < 0)
diff --git a/src/seq.c b/src/seq.c
index ddb63b642..2ca1e4f7b 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -197,7 +197,7 @@ scan_arg (const char *arg)
e = strchr (arg, 'E');
if (e)
{
- long exponent = strtol (e + 1, NULL, 10);
+ long exponent = MAX (strtol (e + 1, NULL, 10), -LONG_MAX);
ret.precision += exponent < 0 ? -exponent
: - MIN (ret.precision, exponent);
/* Don't account for e.... in the width since this is not output. */
diff --git a/tests/misc/numfmt.pl b/tests/misc/numfmt.pl
index a78e9c302..47ff34444 100755
--- a/tests/misc/numfmt.pl
+++ b/tests/misc/numfmt.pl
@@ -178,6 +178,9 @@ my @Tests =
['pad-3.1', '--padding=0 5',
{ERR => "$prog: invalid padding value '0'\n"},
{EXIT => '1'}],
+ ['pad-3.2', "--padding=$limits->{LONG_MIN} 0",
+ {ERR => "$prog: invalid padding value '$limits->{LONG_MIN}'\n"},
+ {EXIT => '1'}],
['pad-4', '--padding=10 --to=si 50000', {OUT=>' 50K'}],
['pad-5', '--padding=-10 --to=si 50000', {OUT=>'50K '}],
diff --git a/tests/misc/seq-precision.sh b/tests/misc/seq-precision.sh
index b5b167783..08f9863ed 100755
--- a/tests/misc/seq-precision.sh
+++ b/tests/misc/seq-precision.sh
@@ -18,6 +18,7 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ seq
+getlimits_
# Integer only. Before v8.24 these would switch output format
@@ -76,4 +77,9 @@ seq -w 1.10000e5 1.10000e5 > out || fail=1
printf "%s\n" 110000 > exp || framework_failure_
compare exp out || fail=1
+# Ensure no undefined behavior which failed with <= 8.32
+# This test would fail with: -fsanitize=undefined
+seq 1e$LONG_MIN 2> err || fail=1
+compare /dev/null err || fail=1
+
Exit $fail
--
2.26.2
