Fix checks for valid integer range subset of 'double'.

Previously, I assumed that checking that a 'double' was between LONG_MIN
and LONG_MAX meant that it would convert to a sensible 'long'.  This is
wrong, because LONG_MAX rounds up to a higher 'double', because it has too
many bits for a 53-bit mantissa.  This could cause problems, like for
example LONG_MAX (as a double) turns out to convert back to LONG_MIN (as a
'long').  This commit fixes the problem.

Thanks to Youngseok Choi for reporting the problem.
Reported at https://lists.gnu.org/archive/html/bug-gnu-pspp/2023-04/msg00005.html.

diff --git a/src/language/commands/matrix.c b/src/language/commands/matrix.c
index 332678160c4211154d7214fcc0d914011bfdecbe..4dd70dcc84ca870b67c961ae4b9fb0f26877a0f6 100644 (file)
--- a/src/language/commands/matrix.c
+++ b/src/language/commands/matrix.c
@@ -47,6 +47,7 @@
 #include "libpspp/array.h"
 #include "libpspp/assertion.h"
 #include "libpspp/compiler.h"
+#include "libpspp/float-range.h"
 #include "libpspp/hmap.h"
 #include "libpspp/i18n.h"
 #include "libpspp/intern.h"
@@ -64,6 +65,7 @@
 #include "gl/c-ctype.h"
 #include "gl/c-strcase.h"
 #include "gl/ftoastr.h"
+#include "gl/intprops.h"
 #include "gl/minmax.h"
 #include "gl/xsize.h"
 
@@ -3385,7 +3387,7 @@ matrix_expr_evaluate_exp_mat (const struct matrix_expr *e,
       return NULL;
     }
   double bf = to_scalar (b);
-  if (bf != floor (bf) || bf <= LONG_MIN || bf > LONG_MAX)
+  if (bf != floor (bf) || bf < DBL_UNIT_LONG_MIN || bf > DBL_UNIT_LONG_MAX)
     {
       msg_at (SE, matrix_expr_location (e->subs[1]),
               _("Exponent %.1f in matrix exponentiation is non-integer "
@@ -3438,11 +3440,30 @@ note_operand_size (const gsl_matrix *m, const struct matrix_expr *e)
           _("This operand is a %zu×%zu matrix."), m->size1, m->size2);
 }
 
+static bool
+is_integer_range (const gsl_matrix *m)
+{
+  if (!is_scalar (m))
+    return false;
+
+  double d = to_scalar (m);
+  return d >= DBL_UNIT_LONG_MIN && d <= DBL_UNIT_LONG_MAX;
+}
+
 static void
-note_nonscalar (const gsl_matrix *m, const struct matrix_expr *e)
+note_noninteger_range (const gsl_matrix *m, const struct matrix_expr *e)
 {
   if (!is_scalar (m))
     note_operand_size (m, e);
+  else
+    {
+      double d = to_scalar (m);
+      if (d < DBL_UNIT_LONG_MIN || d > DBL_UNIT_LONG_MAX)
+        msg_at (SN, matrix_expr_location (e),
+                _("This operand with value %g is outside the supported integer "
+                  "range from %ld to %ld."),
+                d, DBL_UNIT_LONG_MIN, DBL_UNIT_LONG_MAX);
+    }
 }
 
 static gsl_matrix *
@@ -3450,15 +3471,18 @@ matrix_expr_evaluate_seq (const struct matrix_expr *e,
                           gsl_matrix *start_, gsl_matrix *end_,
                           gsl_matrix *by_)
 {
-  if (!is_scalar (start_) || !is_scalar (end_) || (by_ && !is_scalar (by_)))
+  if (!is_integer_range (start_)
+      || !is_integer_range (end_)
+      || (by_ && !is_integer_range (by_)))
     {
       msg_at (SE, matrix_expr_location (e),
-              _("All operands of : operator must be scalars."));
+              _("All operands of : must be scalars in the supported "
+                "integer range."));
 
-      note_nonscalar (start_, e->subs[0]);
-      note_nonscalar (end_, e->subs[1]);
+      note_noninteger_range (start_, e->subs[0]);
+      note_noninteger_range (end_, e->subs[1]);
       if (by_)
-        note_nonscalar (by_, e->subs[2]);
+        note_noninteger_range (by_, e->subs[2]);
       return NULL;
     }
 
@@ -4673,10 +4697,11 @@ matrix_expr_evaluate_integer (const struct matrix_expr *e, const char *context,
     return false;
 
   d = trunc (d);
-  if (d < LONG_MIN || d > LONG_MAX)
+  if (d < DBL_UNIT_LONG_MIN || d > DBL_UNIT_LONG_MAX)
     {
       msg_at (SE, matrix_expr_location (e),
-              _("Expression for %s is outside the integer range."), context);
+              _("Expression for %s is outside the supported integer range."),
+              context);
       return false;
     }
   *integer = d;

diff --git a/src/language/lexer/lexer.c b/src/language/lexer/lexer.c
index 489ade83721db3bac5d80410d906c4141beb4293..152318d5bc6ae4772564c3e3556c8944a8e891fd 100644 (file)
--- a/src/language/lexer/lexer.c
+++ b/src/language/lexer/lexer.c
@@ -37,6 +37,7 @@
 #include "libpspp/assertion.h"
 #include "libpspp/cast.h"
 #include "libpspp/deque.h"
+#include "libpspp/float-range.h"
 #include "libpspp/i18n.h"
 #include "libpspp/intern.h"
 #include "libpspp/ll.h"
@@ -932,6 +933,9 @@ lex_force_int (struct lexer *lexer)
 bool
 lex_force_int_range (struct lexer *lexer, const char *name, long min, long max)
 {
+  min = MAX (min, DBL_UNIT_LONG_MIN);
+  max = MIN (max, DBL_UNIT_LONG_MAX);
+
   bool is_number = lex_is_number (lexer);
   bool is_integer = lex_is_integer (lexer);
   bool too_small = (is_integer ? lex_integer (lexer) < min

diff --git a/src/language/lexer/token.c b/src/language/lexer/token.c
index 124d14638d8f3be3e527173e02dc6f332daff39f..02d19f5d1a0a06d929b4fc5038598488eca009a6 100644 (file)
--- a/src/language/lexer/token.c
+++ b/src/language/lexer/token.c
@@ -25,6 +25,7 @@
 #include "data/identifier.h"
 #include "libpspp/assertion.h"
 #include "libpspp/cast.h"
+#include "libpspp/float-range.h"
 #include "libpspp/misc.h"
 
 #include "gl/ftoastr.h"
@@ -201,14 +202,16 @@ token_print (const struct token *token, FILE *stream)
   putc ('\n', stream);
 }
 
-/* Returns true if T is a numeric token for an integer in the range of "long",
-   except that LONG_MIN is excluded. */
+/* Returns true if T is a numeric token for a 'long' in the unit range of
+   'double'.  LONG_MIN is excluded (usually it's outside the unit range of
+   'double' anyway). */
 bool
 token_is_integer (const struct token *t)
 {
   return (token_is_number (t)
-          && t->number > LONG_MIN
-          && t->number <= LONG_MAX
+          && t->number >= DBL_UNIT_LONG_MIN
+          && t->number <= DBL_UNIT_LONG_MAX
+          && (LONG_MIN < DBL_UNIT_LONG_MIN || t->number > DBL_UNIT_LONG_MIN)
           && floor (t->number) == t->number);
 }
 

diff --git a/src/libpspp/automake.mk b/src/libpspp/automake.mk
index 7a6a4574d90fba3756050606633b97b33310e4e1..a93644488863b6654c0a8462d3ef5eb9f0396df8 100644 (file)
--- a/src/libpspp/automake.mk
+++ b/src/libpspp/automake.mk
@@ -45,6 +45,7 @@ src_libpspp_liblibpspp_la_SOURCES = \
        src/libpspp/ext-array.h \
        src/libpspp/float-format.c \
        src/libpspp/float-format.h \
+       src/libpspp/float-range.h \
        src/libpspp/freaderror.c \
        src/libpspp/freaderror.h \
        src/libpspp/hash-functions.c \

diff --git a/src/libpspp/float-range.h b/src/libpspp/float-range.h
new file mode 100644 (file)
index 0000000..a596e69
--- /dev/null
+++ b/src/libpspp/float-range.h
@@ -0,0 +1,53 @@
+/* PSPP - a program for statistical analysis.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef LIBPSPP_FLOAT_RANGE_H
+#define LIBPSPP_FLOAT_RANGE_H 1
+
+#include <float.h>
+#include <limits.h>
+
+#ifndef LONG_WIDTH
+#error                          /* Defined in C2x <limits.h> */
+#endif
+
+/* Maximum positive integer 'double' represented with no loss of precision
+   (that is, with unit precision).
+
+   The maximum negative integer with this property is -DBL_UNIT_MAX. */
+#if DBL_MANT_DIG == 53          /* 64-bit double */
+#define DBL_UNIT_MAX 9007199254740992.0
+#elif DBL_MANT_DIG == 64        /* 80-bit double */
+#define DBL_UNIT_MAX 18446744073709551616.0
+#elif DBL_MANT_DIG == 113       /* 128-bit double */
+#define DBL_UNIT_MAX 10384593717069655257060992658440192.0
+#else
+#error "Please define DBL_UNIT_MAX for your system (as 2**DBL_MANT_DIG)."
+#endif
+
+/* Intersection of ranges [LONG_MIN,LONG_MAX] and [-DBL_UNIT_MAX,DBL_UNIT_MAX],
+   as a range of 'long's.  This range is the (largest contiguous) set of
+   integer values that can be safely converted between 'long' and 'double'
+   without loss of precision. */
+#if DBL_MANT_DIG < LONG_WIDTH - 1
+#define DBL_UNIT_LONG_MIN ((long) -DBL_UNIT_MAX)
+#define DBL_UNIT_LONG_MAX ((long) DBL_UNIT_MAX)
+#else
+#define DBL_UNIT_LONG_MIN LONG_MIN
+#define DBL_UNIT_LONG_MAX LONG_MAX
+#endif
+
+#endif /* float-range.h */

diff --git a/tests/language/commands/matrix.at b/tests/language/commands/matrix.at
index 5cc52b279203f8e7a52e644f8426d815f1e25859..9ad1608ad6f9f9a325d1242cf1411f1b9fdef898 100644 (file)
--- a/tests/language/commands/matrix.at
+++ b/tests/language/commands/matrix.at
@@ -1567,6 +1567,9 @@ PRINT {-1:-3:0}.
 
 PRINT {1, 2; 3}.
 PRINT {{2; 5}, 3}.
+
+PRINT {{1; 2}:3e50}.
+PRINT {-9223372036854770000:9223372036854775807}.
 END MATRIX.
 ])
 AT_CHECK([pspp matrix.sps], [1], [dnl
@@ -1631,6 +1634,35 @@ matrix.sps:19.8-19.13: note: MATRIX: This operand is a 2×1 matrix.
 matrix.sps:19.16: note: MATRIX: This operand is a 1×1 matrix.
    19 | PRINT {{2; 5}, 3}.
       |                ^
+
+matrix.sps:21.8-21.18: error: MATRIX: All operands of : must be scalars in the
+supported integer range.
+   21 | PRINT {{1; 2}:3e50}.
+      |        ^~~~~~~~~~~
+
+matrix.sps:21.8-21.13: note: MATRIX: This operand is a 2×1 matrix.
+   21 | PRINT {{1; 2}:3e50}.
+      |        ^~~~~~
+
+matrix.sps:21.15-21.18: note: MATRIX: This operand with value 3e+50 is outside
+the supported integer range from -9007199254740992 to 9007199254740992.
+   21 | PRINT {{1; 2}:3e50}.
+      |               ^~~~
+
+matrix.sps:22.8-22.47: error: MATRIX: All operands of : must be scalars in the
+supported integer range.
+   22 | PRINT {-9223372036854770000:9223372036854775807}.
+      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+matrix.sps:22.8-22.27: note: MATRIX: This operand with value -9.22337e+18 is
+outside the supported integer range from -9007199254740992 to 9007199254740992.
+   22 | PRINT {-9223372036854770000:9223372036854775807}.
+      |        ^~~~~~~~~~~~~~~~~~~~
+
+matrix.sps:22.29-22.47: note: MATRIX: This operand with value 9.22337e+18 is
+outside the supported integer range from -9007199254740992 to 9007199254740992.
+   22 | PRINT {-9223372036854770000:9223372036854775807}.
+      |                             ^~~~~~~~~~~~~~~~~~~
 ])
 AT_CLEANUP
 
@@ -3171,7 +3203,7 @@ to scalar, not a 0×0 matrix.
       |             ^~
 
 matrix.sps:81.8-81.12: error: MATRIX: Expression for LOOP is outside the
-integer range.
+supported integer range.
    81 | LOOP i=1e100 to 1e200.
       |        ^~~~~
 ])