* bug#30408: Checking for loss of information on integer conversion
[not found] ` <83y3jq9q4m.fsf@gnu.org>
@ 2018-02-18 20:04 ` Paul Eggert
[not found] ` <74ac7b77-a756-95a9-b490-6952cf106f21@cs.ucla.edu>
2018-03-27 23:19 ` Paul Eggert
2 siblings, 0 replies; 10+ messages in thread
From: Paul Eggert @ 2018-02-18 20:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30408, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2618 bytes --]
Eli Zaretskii wrote:
> Emacs Lisp is not used to write software that controls
> aircraft and spaceships
Actually, I maintain Emacs Lisp code that controls timestamps used in aircraft
and spaceships. I'm not saying that Emacs itself runs the aircraft and
spaceships, but it definitely is used to develop software and data used there.
As luck would have it, I'm currently engaged in an email thread about time
transfer between Earth and Mars (yes, this is really a thing and people are
trying to do it with millisecond precision) that is related to a project where I
regularly use Emacs Lisp. See the thread containing this message:
https://mm.icann.org/pipermail/tz/2018-February/026257.html
> More generally, why signaling an error by default in this case is a
> good idea? ... That would
> be similar to behavior of equivalent constructs in C programs
Sure, and C compilers typically issue diagnostics for situations similar to
what's in Bug#30408. For example, for this C program:
int a = 18446744073709553664;
GCC issues a diagnostic, whereas for the similar Emacs Lisp program:
(setq b 18446744073709553664)
Emacs silently substitutes a number that is off by 2048. It's the latter
behavior that causes the sort of problem seen in Bug#30408.
When people write a floating-point number they naturally expect it to have some
fuzz. But when they write an integer they expect it to be represented exactly,
and not to be rounded. Emacs already reports an overflow error for the
following code that attempts to use the same mathematical value:
(setq c #x10000000000000800)
so it's not like it would be a huge change to do something similar for decimal
integers.
When Emacs was originally developed, its integers were typically 28 bits (not
counting sign) and floating-point numbers could typically represent integers
exactly up to 53 bits (not counting sign), so the old Emacs behavior was
somewhat defensible: although it didn't do bignums, at least it could represent
integers nearly twice as wide as fixnums. However, nowadays Emacs integers
typically have more precision than floating point numbers, and the old Emacs
behavior is more likely to lead to counterintuitive results such as those
described in Bug#30408.
On thinking about it in the light of your comments, I suppose it's confusing
that the proposal used a new signal 'inexact', whereas it should just signal
overflow. After all, that's what string_to_number already does for out-of-range
hexadecimal integers. That issue is easily fixed. Revised patch attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-losing-info-when-converting-integers.patch --]
[-- Type: text/x-patch; name="0001-Avoid-losing-info-when-converting-integers.patch", Size: 7603 bytes --]
From 49895e55ed7ac41dbf3752ab534cd665ef45ee71 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Feb 2018 11:37:22 -0800
Subject: [PATCH] Avoid losing info when converting integers
This fixes some glitches with large integers (Bug#30408).
* doc/lispref/numbers.texi (Integer Basics): Say that
decimal integers out of fixnum range must be representable
exactly as floating-point.
* etc/NEWS: Mention this.
* src/editfns.c (styled_format): Use %.0f when formatting %d or %i
values outside machine integer range, to avoid losing info.
Signal an error for %o or %x values that are too large to be
formatted, to avoid losing info.
* src/lread.c (string_to_number): When converting an integer-format
string to floating-point, signal an error if info is lost.
---
doc/lispref/numbers.texi | 8 +++--
etc/NEWS | 9 +++++
src/editfns.c | 93 ++++++++++++++++++++----------------------------
src/lread.c | 14 ++++++++
4 files changed, 66 insertions(+), 58 deletions(-)
diff --git a/doc/lispref/numbers.texi b/doc/lispref/numbers.texi
index e692ee1cc2..252aafd8fd 100644
--- a/doc/lispref/numbers.texi
+++ b/doc/lispref/numbers.texi
@@ -53,9 +53,11 @@ Integer Basics
chapter assume the minimum integer width of 30 bits.
@cindex overflow
- The Lisp reader reads an integer as a sequence of digits with optional
-initial sign and optional final period. An integer that is out of the
-Emacs range is treated as a floating-point number.
+ The Lisp reader can read an integer as a nonempty sequence of
+decimal digits with optional initial sign and optional final period.
+A decimal integer that is out of the Emacs range is treated as
+floating-point if it can be represented exactly as a floating-point
+number.
@example
1 ; @r{The integer 1.}
diff --git a/etc/NEWS b/etc/NEWS
index 8db638e5ed..36cbcf6500 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -248,6 +248,12 @@ as new-style, bind the new variable 'force-new-style-backquotes' to t.
'cl-struct-define' whose name clashes with a builtin type (e.g.,
'integer' or 'hash-table') now signals an error.
+** When formatting a floating-point number as an octal or hexadecimal
+integer, Emacs now signals an error if the number is too large for the
+implementation to format. When reading an integer outside Emacs
+fixnum range, Emacs now signals an error if the integer cannot be
+represented exactly as a floating-point number. See Bug#30408.
+
\f
* Lisp Changes in Emacs 27.1
@@ -289,6 +295,9 @@ remote systems, which support this check.
If the optional third argument is non-nil, 'make-string' will produce
a multibyte string even if its second argument is an ASCII character.
+** (format "%d" X) no longer mishandles floating-point X values that
+do not fit in a machine integer (Bug#30408).
+
** New JSON parsing and serialization functions 'json-serialize',
'json-insert', 'json-parse-string', and 'json-parse-buffer'. These
are implemented in C using the Jansson library.
diff --git a/src/editfns.c b/src/editfns.c
index 96bb271b2d..d26549ddb8 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4563,32 +4563,30 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
and with pM inserted for integer formats.
At most two flags F can be specified at once. */
char convspec[sizeof "%FF.*d" + max (INT_AS_LDBL, pMlen)];
- {
- char *f = convspec;
- *f++ = '%';
- /* MINUS_FLAG and ZERO_FLAG are dealt with later. */
- *f = '+'; f += plus_flag;
- *f = ' '; f += space_flag;
- *f = '#'; f += sharp_flag;
- *f++ = '.';
- *f++ = '*';
- if (float_conversion)
- {
- if (INT_AS_LDBL)
- {
- *f = 'L';
- f += INTEGERP (arg);
- }
- }
- else if (conversion != 'c')
- {
- memcpy (f, pMd, pMlen);
- f += pMlen;
- zero_flag &= ! precision_given;
- }
- *f++ = conversion;
- *f = '\0';
- }
+ char *f = convspec;
+ *f++ = '%';
+ /* MINUS_FLAG and ZERO_FLAG are dealt with later. */
+ *f = '+'; f += plus_flag;
+ *f = ' '; f += space_flag;
+ *f = '#'; f += sharp_flag;
+ *f++ = '.';
+ *f++ = '*';
+ if (float_conversion)
+ {
+ if (INT_AS_LDBL)
+ {
+ *f = 'L';
+ f += INTEGERP (arg);
+ }
+ }
+ else if (conversion != 'c')
+ {
+ memcpy (f, pMd, pMlen);
+ f += pMlen;
+ zero_flag &= ! precision_given;
+ }
+ *f++ = conversion;
+ *f = '\0';
int prec = -1;
if (precision_given)
@@ -4630,29 +4628,18 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
}
else if (conversion == 'd' || conversion == 'i')
{
- /* For float, maybe we should use "%1.0f"
- instead so it also works for values outside
- the integer range. */
- printmax_t x;
if (INTEGERP (arg))
- x = XINT (arg);
+ {
+ printmax_t x = XINT (arg);
+ sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
+ }
else
{
- double d = XFLOAT_DATA (arg);
- if (d < 0)
- {
- x = TYPE_MINIMUM (printmax_t);
- if (x < d)
- x = d;
- }
- else
- {
- x = TYPE_MAXIMUM (printmax_t);
- if (d < x)
- x = d;
- }
+ strcpy (f - pMlen - 1, "f");
+ prec = 0;
+ double x = XFLOAT_DATA (arg);
+ sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
}
- sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
}
else
{
@@ -4663,22 +4650,18 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
else
{
double d = XFLOAT_DATA (arg);
- if (d < 0)
- x = 0;
- else
- {
- x = TYPE_MAXIMUM (uprintmax_t);
- if (d < x)
- x = d;
- }
+ if (! (0 <= d && d < TYPE_MAXIMUM (uprintmax_t)))
+ xsignal1 (Qoverflow_error, arg);
+ x = d;
}
sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
}
/* Now the length of the formatted item is known, except it omits
padding and excess precision. Deal with excess precision
- first. This happens only when the format specifies
- ridiculously large precision. */
+ first. This happens when the format specifies
+ ridiculously large precision, or when %d or %i has
+ nonzero precision and formats a float. */
ptrdiff_t excess_precision
= precision_given ? precision - prec : 0;
ptrdiff_t leading_zeros = 0, trailing_zeros = 0;
diff --git a/src/lread.c b/src/lread.c
index d009bd0cd2..9500ed8341 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -3794,6 +3794,20 @@ string_to_number (char const *string, int base, bool ignore_trailing)
if (! value)
value = atof (string + signedp);
+ if (! float_syntax)
+ {
+ /* Check that converting the integer-format STRING to a
+ floating-point number does not lose info. See Bug#30408. */
+ char const *bp = string + signedp;
+ while (*bp == '0')
+ bp++;
+ char checkbuf[DBL_MAX_10_EXP + 2];
+ int checkbuflen = sprintf (checkbuf, "%.0f", value);
+ if (! (cp - bp - !!(state & DOT_CHAR) == checkbuflen
+ && memcmp (bp, checkbuf, checkbuflen) == 0))
+ xsignal1 (Qoverflow_error, build_string (string));
+ }
+
return make_float (negative ? -value : value);
}
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#30408: Checking for loss of information on integer conversion
[not found] ` <83y3jq9q4m.fsf@gnu.org>
2018-02-18 20:04 ` bug#30408: Checking for loss of information on integer conversion Paul Eggert
[not found] ` <74ac7b77-a756-95a9-b490-6952cf106f21@cs.ucla.edu>
@ 2018-03-27 23:19 ` Paul Eggert
2018-03-29 11:11 ` Eli Zaretskii
2 siblings, 1 reply; 10+ messages in thread
From: Paul Eggert @ 2018-03-27 23:19 UTC (permalink / raw)
To: Eli Zaretskii, 30408
[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]
Here's a patch that I hope addresses the main problem. The basic idea is
to avoid the confusion exemplified in Bug#30408 by changing Emacs so
that it ordinarily signals an error if it reads a program that contains
an integer literal that is out of fixnum range. However, if the
out-of-range literal is followed by '.' then Emacs continues to silently
convert it to floating-point; this is intended as an escape hatch for
any programs that need the old behavior (I expect this'll be rare).
Thus, on 32-bit Emacs, plain '536870912' in a program causes Emacs to
signal an overflow while loading the program, whereas '536870912.' is
treated as a floating-point number as before. (On 64-bit Emacs, the same
two literals are both integers, as before.)
Unlike my previous proposal, this patch does not affect the behavior of
string-to-integer. As I understand it, that was a primary source of
qualms about the previous proposal.
I've tested this on both 32- and 64-bit Emacs on master. This patch has
helped me to find a couple of integer portability bugs which I already
fixed on master.
[-- Attachment #2: 0001-Lisp-reader-now-checks-for-integer-overflow.patch --]
[-- Type: text/x-patch, Size: 11154 bytes --]
From 94b7a1a171de3113cd5250315dee7bdef5f51890 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 27 Mar 2018 15:48:47 -0700
Subject: [PATCH] Lisp reader now checks for integer overflow
* doc/lispref/numbers.texi (Integer Basics), etc/NEWS:
Document this.
* src/lisp.h (S2N_IGNORE_TRAILING, S2N_OVERFLOW_TO_FLOAT):
New constants.
* src/lread.c (string_to_number): Change trailing bool arg to
integer argument with flags, to support S2N_OVERFLOW_TO_FLOAT.
All uses changed.
* test/src/editfns-tests.el (read-large-integer): New test.
---
doc/lispref/numbers.texi | 14 ++++++++++----
etc/NEWS | 7 +++++++
src/data.c | 9 ++++-----
src/lisp.h | 3 ++-
src/lread.c | 35 ++++++++++++++++++++---------------
src/process.c | 2 +-
test/src/editfns-tests.el | 22 ++++++++++++++++++----
7 files changed, 62 insertions(+), 30 deletions(-)
diff --git a/doc/lispref/numbers.texi b/doc/lispref/numbers.texi
index c2cb6651d4..2fed2b642f 100644
--- a/doc/lispref/numbers.texi
+++ b/doc/lispref/numbers.texi
@@ -55,16 +55,13 @@ Integer Basics
The Lisp reader reads an integer as a nonempty sequence
of decimal digits with optional initial sign and optional
-final period. A decimal integer that is out of the
-Emacs range is treated as a floating-point number.
+final period.
@example
1 ; @r{The integer 1.}
1. ; @r{The integer 1.}
+1 ; @r{Also the integer 1.}
-1 ; @r{The integer @minus{}1.}
- 9000000000000000000
- ; @r{The floating-point number 9e18.}
0 ; @r{The integer 0.}
-0 ; @r{The integer 0.}
@end example
@@ -94,6 +91,15 @@ Integer Basics
#24r1k @result{} 44
@end example
+ If an integer is outside the Emacs range, the Lisp reader ordinarily
+signals an overflow. However, if a too-large plain integer ends in a
+period, the Lisp reader treats it as a floating-point number instead.
+This lets an Emacs Lisp program specify a large integer that is
+quietly approximated by a floating-point number on machines with
+limited word width. For example, @samp{536870912.} is a
+floating-point number if Emacs integers are only 30 bits wide and is
+an integer otherwise.
+
To understand how various functions work on integers, especially the
bitwise operators (@pxref{Bitwise Operations}), it is often helpful to
view the numbers in their binary form.
diff --git a/etc/NEWS b/etc/NEWS
index fd1d04b8a0..cb74f512cc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -349,6 +349,13 @@ as new-style, bind the new variable 'force-new-style-backquotes' to t.
integer, Emacs now signals an error if the number is too large for the
implementation to format (Bug#30408).
++++
+** The Lisp reader now signals an overflow for plain decimal integers
+that do not end in '.' and are outside Emacs range. Formerly the Lisp
+reader silently converted them to floating-point numbers, and signaled
+overflow only for integers with a radix that are outside machine range
+(Bug#30408).
+
---
** Some functions and variables obsolete since Emacs 22 have been removed:
archive-mouse-extract, assoc-ignore-case, assoc-ignore-representation,
diff --git a/src/data.c b/src/data.c
index a7fab1ef58..6f23a26757 100644
--- a/src/data.c
+++ b/src/data.c
@@ -2716,9 +2716,7 @@ present, base 10 is used. BASE must be between 2 and 16 (inclusive).
If the base used is not 10, STRING is always parsed as an integer. */)
(register Lisp_Object string, Lisp_Object base)
{
- register char *p;
- register int b;
- Lisp_Object val;
+ int b;
CHECK_STRING (string);
@@ -2732,11 +2730,12 @@ If the base used is not 10, STRING is always parsed as an integer. */)
b = XINT (base);
}
- p = SSDATA (string);
+ char *p = SSDATA (string);
while (*p == ' ' || *p == '\t')
p++;
- val = string_to_number (p, b, true);
+ int flags = S2N_IGNORE_TRAILING | S2N_OVERFLOW_TO_FLOAT;
+ Lisp_Object val = string_to_number (p, b, flags);
return NILP (val) ? make_number (0) : val;
}
\f
diff --git a/src/lisp.h b/src/lisp.h
index f0c0c5a14a..b931d23bf3 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3899,7 +3899,8 @@ LOADHIST_ATTACH (Lisp_Object x)
}
extern int openp (Lisp_Object, Lisp_Object, Lisp_Object,
Lisp_Object *, Lisp_Object, bool);
-extern Lisp_Object string_to_number (char const *, int, bool);
+enum { S2N_IGNORE_TRAILING = 1, S2N_OVERFLOW_TO_FLOAT = 2 };
+extern Lisp_Object string_to_number (char const *, int, int);
extern void map_obarray (Lisp_Object, void (*) (Lisp_Object, Lisp_Object),
Lisp_Object);
extern void dir_warning (const char *, Lisp_Object);
diff --git a/src/lread.c b/src/lread.c
index 381f9cf20c..a774524ee4 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2339,7 +2339,7 @@ character_name_to_code (char const *name, ptrdiff_t name_len)
monstrosities like "U+-0000". */
Lisp_Object code
= (name[0] == 'U' && name[1] == '+'
- ? string_to_number (name + 1, 16, false)
+ ? string_to_number (name + 1, 16, 0)
: call2 (Qchar_from_name, make_unibyte_string (name, name_len), Qt));
if (! RANGED_INTEGERP (0, code, MAX_UNICODE_CHAR)
@@ -2693,7 +2693,7 @@ read_integer (Lisp_Object readcharfun, EMACS_INT radix)
invalid_syntax (buf);
}
- return string_to_number (buf, radix, false);
+ return string_to_number (buf, radix, 0);
}
@@ -3502,7 +3502,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
if (!quoted && !uninterned_symbol)
{
- Lisp_Object result = string_to_number (read_buffer, 10, false);
+ Lisp_Object result = string_to_number (read_buffer, 10, 0);
if (! NILP (result))
return unbind_to (count, result);
}
@@ -3667,16 +3667,17 @@ substitute_in_interval (INTERVAL interval, void *arg)
}
\f
-/* Convert STRING to a number, assuming base BASE. Return a fixnum if
- STRING has integer syntax and fits in a fixnum, else return the
- nearest float if STRING has either floating point or integer syntax
- and BASE is 10, else return nil. If IGNORE_TRAILING, consider just
- the longest prefix of STRING that has valid floating point syntax.
- Signal an overflow if BASE is not 10 and the number has integer
- syntax but does not fit. */
+/* Convert STRING to a number, assuming base BASE. When STRING has
+ floating point syntax and BASE is 10, return a nearest float. When
+ STRING has integer syntax, return a fixnum if the integer fits, and
+ signal an overflow otherwise (unless BASE is 10 and STRING ends in
+ period or FLAGS & S2N_OVERFLOW_TO_FLOAT is nonzero; in this case,
+ return a nearest float instead). Otherwise, return nil. If FLAGS
+ & S2N_IGNORE_TRAILING is nonzero, consider just the longest prefix
+ of STRING that has valid syntax. */
Lisp_Object
-string_to_number (char const *string, int base, bool ignore_trailing)
+string_to_number (char const *string, int base, int flags)
{
char const *cp = string;
bool float_syntax = 0;
@@ -3759,9 +3760,10 @@ string_to_number (char const *string, int base, bool ignore_trailing)
|| (state & ~INTOVERFLOW) == (LEAD_INT|E_EXP));
}
- /* Return nil if the number uses invalid syntax. If IGNORE_TRAILING, accept
- any prefix that matches. Otherwise, the entire string must match. */
- if (! (ignore_trailing
+ /* Return nil if the number uses invalid syntax. If FLAGS &
+ S2N_IGNORE_TRAILING, accept any prefix that matches. Otherwise,
+ the entire string must match. */
+ if (! (flags & S2N_IGNORE_TRAILING
? ((state & LEAD_INT) != 0 || float_syntax)
: (!*cp && ((state & ~(INTOVERFLOW | DOT_CHAR)) == LEAD_INT
|| float_syntax))))
@@ -3776,7 +3778,7 @@ string_to_number (char const *string, int base, bool ignore_trailing)
/* Unfortunately there's no simple and accurate way to convert
non-base-10 numbers that are out of C-language range. */
if (base != 10)
- xsignal1 (Qoverflow_error, build_string (string));
+ flags = 0;
}
else if (n <= (negative ? -MOST_NEGATIVE_FIXNUM : MOST_POSITIVE_FIXNUM))
{
@@ -3785,6 +3787,9 @@ string_to_number (char const *string, int base, bool ignore_trailing)
}
else
value = n;
+
+ if (! (state & DOT_CHAR) && ! (flags & S2N_OVERFLOW_TO_FLOAT))
+ xsignal1 (Qoverflow_error, build_string (string));
}
/* Either the number uses float syntax, or it does not fit into a fixnum.
diff --git a/src/process.c b/src/process.c
index 2aaa238f60..ed2cab7b51 100644
--- a/src/process.c
+++ b/src/process.c
@@ -6842,7 +6842,7 @@ SIGCODE may be an integer, or a symbol whose name is a signal name. */)
{
Lisp_Object tem = Fget_process (process);
if (NILP (tem))
- tem = string_to_number (SSDATA (process), 10, true);
+ tem = string_to_number (SSDATA (process), 10, S2N_OVERFLOW_TO_FLOAT);
process = tem;
}
else if (!NUMBERP (process))
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index 6e1f730166..442ad08937 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -142,27 +142,41 @@ transpose-test-get-byte-positions
(should (string-equal (format "%#05X" #x10) "0X010"))
(should (string-equal (format "%#04x" 0) "0000")))
-;;; Test Bug#30408.
+
+;;; Tests for Bug#30408.
+
(ert-deftest format-%d-large-float ()
(should (string-equal (format "%d" 18446744073709551616.0)
"18446744073709551616"))
(should (string-equal (format "%d" -18446744073709551616.0)
"-18446744073709551616")))
-;;; Another test for Bug#30408.
;;; Perhaps Emacs will be improved someday to return the correct
;;; answer for positive numbers instead of overflowing; in
-;;; that case this test will need to be changed. In the meantime make
+;;; that case these tests will need to be changed. In the meantime make
;;; sure Emacs is reporting the overflow correctly.
(ert-deftest format-%x-large-float ()
(should-error (format "%x" 18446744073709551616.0)
:type 'overflow-error))
+(ert-deftest read-large-integer ()
+ (should-error (read (format "%d0" most-negative-fixnum))
+ :type 'overflow-error)
+ (should-error (read (format "%+d" (* -8.0 most-negative-fixnum)))
+ :type 'overflow-error)
+ (should-error (read (substring (format "%d" most-negative-fixnum) 1))
+ :type 'overflow-error)
+ (should-error (read (format "#x%x" most-negative-fixnum))
+ :type 'overflow-error)
+ (should-error (read (format "#o%o" most-negative-fixnum))
+ :type 'overflow-error)
+ (should-error (read (format "#32rG%x" most-positive-fixnum))
+ :type 'overflow-error))
-;;; Another test for Bug#30408.
(ert-deftest format-%o-invalid-float ()
(should-error (format "%o" -1e-37)
:type 'overflow-error))
+
;;; Check format-time-string with various TZ settings.
;;; Use only POSIX-compatible TZ values, since the tests should work
;;; even if tzdb is not in use.
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread