Follow-up Comment #1, bug #66001 (group groff): I think I've got it licked. All tests passing.
commit bee5cf5dc98600efe48b96014572be9c056419c3
Author: G. Branden Robinson <g.branden.robin...@gmail.com>
Date: Fri Aug 16 14:53:13 2024 -0500
[troff]: Fix Savannah #66001 (saturating arith).
* src/roff/troff/hvunits.h (vunits::to_units)
(class vunits: operator +, operator -, operator *):
(hunits::to_units)
(class hunits: operator +, operator -, operator *):
* src/roff/troff/number.cpp (vunits::vunits, hunits::hunits): Impose
saturating arithmetic on overflowing operations. Demote overflow from
error, throwing warning in "range" category if overflow would occur,
and describe integer result as "saturated" rather than "wrapped".
* src/roff/groff/tests/arithmetic-works.sh: Enable warnings in "range"
category.
* doc/groff.texi.in (Numeric Expressions):
* man/groff.7.man (Numeric expressions):
* NEWS: Document it.
Fixes <https://savannah.gnu.org/bugs/?66001>.
[I decided against reverting commit 19e6016e89, 31 July. The test
script has evolved since then.]
diff --git a/ChangeLog b/ChangeLog
index a4df1c4ec..3e5432d34 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2024-08-16 G. Branden Robinson <g.branden.robin...@gmail.com>
+
+ * src/roff/troff/hvunits.h (vunits::to_units)
+ (class vunits: operator +, operator -, operator *):
+ (hunits::to_units)
+ (class hunits: operator +, operator -, operator *):
+ * src/roff/troff/number.cpp (vunits::vunits, hunits::hunits):
+ Impose saturating arithmetic on overflowing operations. Demote
+ overflow from error, throwing warning in "range" category if
+ overflow would occur, and describe integer result as "saturated"
+ rather than "wrapped".
+
+ * src/roff/groff/tests/arithmetic-works.sh: Enable warnings in
+ "range" category.
+
+ * doc/groff.texi.in (Numeric Expressions):
+ * man/groff.7.man (Numeric expressions):
+ * NEWS: Document it.
+
+ Fixes <https://savannah.gnu.org/bugs/?66001>.
+
2024-08-16 G. Branden Robinson <g.branden.robin...@gmail.com>
* src/roff/troff/number.cpp (get_vunits, get_hunts, get_number)
diff --git a/NEWS b/NEWS
index 48abf9a10..1e1f3d864 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,9 @@ o The `color`, `cp`, `linetabs`, and `vpt` requests now
interpret
with a register interpolation as an argument, the outcome agrees with
an `if` test of the register's value.
+o GNU troff now implements saturating rather than wrapping integer
+ arithmetic.
+
o The formatter no longer reports warnings in the "el" category. There
was only one; it threw a warning diagnostic if enabled (which it was
not by default) if it inferred an imbalance between `ie` and `el`
diff --git a/doc/groff.texi.in b/doc/groff.texi.in
index f51e66a35..93b316c2a 100644
--- a/doc/groff.texi.in
+++ b/doc/groff.texi.in
@@ -6189,7 +6189,8 @@ @node Numeric Expressions
32-bit signed integer, which suffices to represent magnitudes within a
range of <B1>2 billion.@footnote{If that's not enough, see the
@cite{groff_tmac@r{(5)}} man page for the @file{62bit.tmac} macro
-package.}
+package.} Arithmetic saturates rather than wrapping; if overflow would
+occur, GNU @command{troff} emits a warning in category @samp{range}.
@cindex arithmetic operators
@cindex operators, arithmetic
diff --git a/man/groff.7.man b/man/groff.7.man
index b3c1ef163..37083245d 100644
--- a/man/groff.7.man
+++ b/man/groff.7.man
@@ -976,6 +976,12 @@ .SH "Numeric expressions"
.I 62bit.tmac
macro package.)
.
+Arithmetic saturates rather than wrapping;
+if overflow would occur,
+.I @g@troff
+emits a warning in category
+.RB \%\[lq] range \[rq].
+.
.
.P
Arithmetic infix operators perform a function on the numeric expressions
diff --git a/src/roff/groff/tests/arithmetic-works.sh
b/src/roff/groff/tests/arithmetic-works.sh
index f7bb699ad..f5a10a054 100755
--- a/src/roff/groff/tests/arithmetic-works.sh
+++ b/src/roff/groff/tests/arithmetic-works.sh
@@ -57,13 +57,13 @@ input='.
#
# 2: .l=1512
#
-# 3: .l=24, .H=24
+# 3: .l=2147483640, .H=24
#
-# 4: .p=40, .V=40
+# 4: .p=2147483640, .V=40
#
# The blank lines are due to the `vs` increase.
-output=$(echo "$input" | "$groff" -T ascii)
+output=$(echo "$input" | "$groff" -w range -T ascii)
echo "$output"
echo "checking that vertical spacing is correctly incremented" >&2
@@ -72,11 +72,11 @@ echo "$output" | grep -Fqx '1: .v=80' || wail
echo "checking that line length is correctly incremented" >&2
echo "$output" | grep -Fqx '2: .l=1512' || wail
-echo "checking that setting huge line length does not overflow" >&2
-echo "$output" | grep -Fqx '3: .l=24, .H=24' || wail
+echo "checking that setting huge line length saturates" >&2
+echo "$output" | grep -Fqx '3: .l=2147483640, .H=24' || wail
-echo "checking that setting huge page length does not overflow" >&2
-echo "$output" | grep -Fqx '4: .p=40, .V=40' || wail
+echo "checking that setting huge page length saturates" >&2
+echo "$output" | grep -Fqx '4: .p=2147483640, .V=40' || wail
input='.
.ec @
@@ -91,7 +91,7 @@ c: @nc
d: @nd
.'
-output=$(echo "$input" | "$groff" -T ascii)
+output=$(echo "$input" | "$groff" -w range -T ascii)
echo "$output"
# The vunits and hunits constructors in src/roff/troff/number.cpp don't
@@ -137,7 +137,7 @@ input='.
7: (-1)*(-2147483648) -> @ni
.'
-output=$(echo "$input" | "$groff" -T ascii)
+output=$(echo "$input" | "$groff" -w range -T ascii)
echo "$output"
# 2147483647
diff --git a/src/roff/troff/hvunits.h b/src/roff/troff/hvunits.h
index a6f6fb456..63de9c405 100644
--- a/src/roff/troff/hvunits.h
+++ b/src/roff/troff/hvunits.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2024 Free Software Foundation, Inc.
Written by James Clark (j...@jclark.com)
This file is part of groff.
@@ -93,7 +93,7 @@ inline units vunits::to_units()
{
units r;
if (ckd_mul(&r, n, vresolution))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
@@ -107,7 +107,7 @@ inline vunits operator +(const vunits & x, const vunits &
y)
vunits r;
r = x;
if (ckd_add(&r.n, r.n, y.n))
- error("integer addition wrapped");
+ warning(WARN_RANGE, "integer addition saturated");
return r;
}
@@ -116,7 +116,7 @@ inline vunits operator -(const vunits & x, const vunits &
y)
vunits r;
r = x;
if (ckd_sub(&r.n, r.n, y.n))
- error("integer subtraction wrapped");
+ warning(WARN_RANGE, "integer subtraction saturated");
return r;
}
@@ -125,7 +125,7 @@ inline vunits operator -(const vunits & x)
vunits r;
// Why? Consider -(INT_MIN) in two's complement.
if (ckd_mul(&r.n, x.n, -1))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
@@ -147,7 +147,7 @@ inline vunits operator *(const vunits & x, int n)
vunits r;
r = x;
if (ckd_mul(&r.n, x.n, n))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
@@ -156,7 +156,7 @@ inline vunits operator *(int n, const vunits & x)
vunits r;
r = x;
if (ckd_mul(&r.n, n, x.n))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
@@ -210,7 +210,7 @@ inline units hunits::to_units()
{
units r;
if (ckd_mul(&r, n, hresolution))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
@@ -224,7 +224,7 @@ inline hunits operator +(const hunits & x, const hunits &
y)
hunits r;
r = x;
if (ckd_add(&r.n, r.n, y.n))
- error("integer addition wrapped");
+ warning(WARN_RANGE, "integer addition saturated");
return r;
}
@@ -233,7 +233,7 @@ inline hunits operator -(const hunits & x, const hunits &
y)
hunits r;
r = x;
if (ckd_sub(&r.n, r.n, y.n))
- error("integer subtraction wrapped");
+ warning(WARN_RANGE, "integer subtraction saturated");
return r;
}
@@ -243,7 +243,7 @@ inline hunits operator -(const hunits & x)
r = x;
// Why? Consider -(INT_MIN) in two's complement.
if (ckd_mul(&r.n, x.n, -1))
- error("integer subtraction wrapped");+ warning(WARN_RANGE, "integer
subtraction saturated");
return r;
}
@@ -265,7 +265,7 @@ inline hunits operator *(const hunits & x, int n)
hunits r;
r = x;
if (ckd_mul(&r.n, x.n, n))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
@@ -274,7 +274,7 @@ inline hunits operator *(int n, const hunits & x)
hunits r;
r = x;
if (ckd_mul(&r.n, x.n, n))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return r;
}
diff --git a/src/roff/troff/number.cpp b/src/roff/troff/number.cpp
index b887501c7..cdaca35f9 100644
--- a/src/roff/troff/number.cpp
+++ b/src/roff/troff/number.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2024 Free Software Foundation, Inc.
Written by James Clark (j...@jclark.com)
This file is part of groff.
@@ -129,12 +129,12 @@ bool get_vunits(vunits *res, unsigned char si, vunits
prev_value)
break;
case INCREMENT:
if (ckd_add(&i, prev_value.to_units(), v))
- error("integer incrementation wrapped");
+ warning(WARN_RANGE, "integer incrementation saturated");
*res = i;
break;
case DECREMENT:
if (ckd_sub(&i, prev_value.to_units(), v))
- error("integer subtraction wrapped");
+ warning(WARN_RANGE, "integer subtraction saturated");
*res = i;
break;
default:
@@ -157,12 +157,12 @@ bool get_hunits(hunits *res, unsigned char si, hunits
prev_value)
break;
case INCREMENT:
if (ckd_add(&i, prev_value.to_units(), h))
- error("integer incrementation wrapped");
+ warning(WARN_RANGE, "integer incrementation saturated");
*res = i;
break;
case DECREMENT:
if (ckd_sub(&i, prev_value.to_units(), h))
- error("integer decrementation wrapped");
+ warning(WARN_RANGE, "integer decrementation saturated");
*res = i;
break;
default:
@@ -182,11 +182,11 @@ bool get_number(units *res, unsigned char si, units
prev_value)
break;
case INCREMENT:
if (ckd_add(res, prev_value, u))
- error("integer incrementation wrapped");
+ warning(WARN_RANGE, "integer incrementation saturated");
break;
case DECREMENT:
if (ckd_sub(res, prev_value, u))
- error("integer subtraction wrapped");
+ warning(WARN_RANGE, "integer subtraction saturated");
break;
default:
assert(0 == "unhandled case in get_number()");
@@ -205,11 +205,11 @@ bool get_integer(int *res, int prev_value)
break;
case INCREMENT:
if (ckd_add(res, prev_value, i))
- error("integer incrementation wrapped");
+ warning(WARN_RANGE, "integer incrementation saturated");
break;
case DECREMENT:
if (ckd_sub(res, prev_value, i))
- error("integer subtraction wrapped");
+ warning(WARN_RANGE, "integer subtraction saturated");
break;
default:
assert(0 == "unhandled case in get_integer()");
@@ -351,19 +351,19 @@ static bool is_valid_expression(units *u, int
scaling_unit,
break;
case '+':
if (ckd_add(u, *u, u2)) {
- error("integer addition wrapped");
+ warning(WARN_RANGE, "integer addition saturated");
return false;
}
break;
case '-':
if (ckd_sub(u, *u, u2)) {
- error("integer subtraction wrapped");
+ warning(WARN_RANGE, "integer subtraction saturated");
return false;
}
break;
case '*':
if (ckd_mul(u, *u, u2)) {
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
return false;
}
break;
@@ -466,7 +466,7 @@ static bool is_valid_term(units *u, int scaling_unit,
if (is_negative) {
// Why? Consider -(INT_MIN) in two's complement.
if (ckd_mul(u, *u, -1))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
}
return true;
case '.':
@@ -495,7 +495,7 @@ static bool is_valid_term(units *u, int scaling_unit,
c = tok.ch();
} while (csdigit(c));
if (is_overflowing)
- error("integer value wrapped");
+ warning(WARN_RANGE, "integer value saturated");
break;
case '/':
case '*':
@@ -623,7 +623,7 @@ static bool is_valid_term(units *u, int scaling_unit,
tok.next();
if (is_negative) {
if (ckd_mul(u, *u, -1))
- error("integer multiplication wrapped");
+ warning(WARN_RANGE, "integer multiplication saturated");
}
return true;
}
@@ -671,9 +671,17 @@ vunits::vunits(units x)
if (ckd_add(&n, x, vcrement))
is_overflowing = true;
}
+ if (is_overflowing) {
+ if (x < 0) {
+ warning(WARN_RANGE, "negative integer value saturated");
+ n = INT_MIN;
+ }
+ else {
+ warning(WARN_RANGE, "positive integer value saturated");
+ n = INT_MAX;
+ }
+ }
n /= vresolution;
- if (is_overflowing)
- error("integer addition wrapped");
}
}
@@ -694,9 +702,17 @@ hunits::hunits(units x)
if (ckd_add(&n, x, hcrement))
is_overflowing = true;
}
+ if (is_overflowing) {
+ if (x < 0) {
+ warning(WARN_RANGE, "negative integer value saturated");
+ n = INT_MIN;
+ }
+ else {
+ warning(WARN_RANGE, "positive integer value saturated");
+ n = INT_MAX;
+ }
+ }
n /= hresolution;
- if (is_overflowing)
- error("integer addition wrapped");
}
}
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/bugs/?66001>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
signature.asc
Description: PGP signature
