From: Ben Pfaff Date: Sun, 23 Apr 2023 01:05:44 +0000 (-0700) Subject: Fix checks for valid integer range subset of 'double'. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=505a04fe4cf5cb6f81b498e997898e6069387791;p=pspp 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 332678160c..4dd70dcc84 100644 --- 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 489ade8372..152318d5bc 100644 --- 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 124d14638d..02d19f5d1a 100644 --- 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 7a6a4574d9..a936444888 100644 --- 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 index 0000000000..a596e69c5f --- /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 . */ + +#ifndef LIBPSPP_FLOAT_RANGE_H +#define LIBPSPP_FLOAT_RANGE_H 1 + +#include +#include + +#ifndef LONG_WIDTH +#error /* Defined in C2x */ +#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 5cc52b2792..9ad1608ad6 100644 --- 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. | ^~~~~ ])