Date: Sun, 20 Apr 2003 04:21:38 +0400 (MSD) From: Alex Semenyaka <alexs@snark.ratmir.ru> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/51171: /bin/sh has only 32-bit arithmetics that is not enough [PATCH INCLUDED] Message-ID: <200304200021.h3K0LcFv052208@snark.ratmir.ru> Resent-Message-ID: <200304200030.h3K0UKg5050308@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 51171 >Category: bin >Synopsis: /bin/sh has only 32-bit arithmetics that is not enough [PATCH INCLUDED] >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Apr 19 17:30:20 PDT 2003 >Closed-Date: >Last-Modified: >Originator: Alex Semenyaka >Release: FreeBSD 4.8-RC i386 >Organization: Ratmir >Environment: System: FreeBSD snark.ratmir.ru 4.8-RC FreeBSD 4.8-RC #7: Sun Mar 30 07:23:48 MSD 2003 root@snark.ratmir.ru:/usr/obj/usr/src/sys/SNARK i386 >Description: /bin/sh handles only 32-bit numbers while there are a lot of 64-bit values in the system now. Also it just silently ignores hte overflows so the system administrator have no chance to find out that there were incorrect calculations done in the scripts. It is traditional behaviour but not necessary. Here is my arguments I've send to -standards mailing list. They did not meet objections: 1) POSIX and SUSv3 _require_ at least long type for integers in the shell, and explicitly _allow_ to use integer in the range beyond the long type range. SUSv2 _requires_ long and _allows_ extensions as well (but there are no explicit words about wider range as in SUSv3). It is the theoretical aspect. 2) ksh, zsh and bash handle longer integers properly. The last two have 64-bit arithmetics and first one has float inside. That is the compatibility aspect. 3) These days we have a lot of 64-bit stuff in the system: from ipfw counters to file sizes. Moreover we really have such big numbers in everyday FreeBSD usage. But now /bin/sh silently produce just wrong results dealing with such numbers due to overflow. So old scripts which worked from 1997 or such now can produce meaningless results and it will not be even noticed. I can suggest to write new ones with bash or zsh but some people continue to use their old stuff (don't touch while works, you know). So it IS better to change our basic shell and extend it's abilities. That is the practical aspect. >How-To-Repeat: $ /bin/sh -c 'echo $((12345678900))' 2147483647 >Fix: I suggest the following patch on the base of the discussion in the -hackers. It introduces 64-bit arithmetics. Also it adds a new command-line option: -O (error when there is overflow). It is possible to build /bin/sh without the overflow control, just -DOVERFLOW needed to be removed from the Makefile. diff -u -r -U 2 -b ../sh.old/Makefile ./Makefile --- ../sh.old/Makefile Sat Apr 19 23:22:31 2003 +++ ./Makefile Sun Apr 20 02:47:41 2003 @@ -19,5 +19,5 @@ LFLAGS= -8 # 8-bit lex scanner for arithmetic -CFLAGS+=-DSHELL -I. -I${.CURDIR} +CFLAGS+=-DSHELL -I. -I${.CURDIR} -DOVERFLOW # for debug: #CFLAGS+= -g -DDEBUG=2 -pg diff -u -r -U 2 -b ../sh.old/arith.h ./arith.h --- ../sh.old/arith.h Fri Jul 19 08:38:51 2002 +++ ./arith.h Sun Apr 20 02:37:37 2003 @@ -35,4 +35,7 @@ */ -int arith(char *); +/* XXX some day probably should go to /usr/include/machine/_inttypes.h */ +#define MAXINT_LEN 20 + +intmax_t arith(char *); int expcmd(int , char **); diff -u -r -U 2 -b ../sh.old/arith.y ./arith.y --- ../sh.old/arith.y Fri Jul 19 08:38:51 2002 +++ ./arith.y Sun Apr 20 03:52:31 2003 @@ -1,2 +1,13 @@ +%{ +#include <stdint.h> +#ifdef OVERFLOW +#include "options.h" +#endif + +#define YYSTYPE intmax_t + +static intmax_t arith_res; +%} + %token ARITH_NUM ARITH_LPAREN ARITH_RPAREN @@ -15,5 +26,6 @@ exp: expr = { - return ($1); + arith_res = $1; + return (0); } ; @@ -34,7 +46,25 @@ | expr ARITH_LSHIFT expr = { $$ = $1 << $3; } | expr ARITH_RSHIFT expr = { $$ = $1 >> $3; } - | expr ARITH_ADD expr = { $$ = $1 + $3; } - | expr ARITH_SUB expr = { $$ = $1 - $3; } - | expr ARITH_MUL expr = { $$ = $1 * $3; } + | expr ARITH_ADD expr = { + $$ = $1 + $3; +#ifdef OVERFLOW + if (Oflag && is_add_overflow($1, $3, $$)) + yyerror("overflow in"); +#endif + } + | expr ARITH_SUB expr = { + $$ = $1 - $3; +#ifdef OVERFLOW + if (Oflag && is_add_overflow($1, -$3, $$)) + yyerror("overflow in"); +#endif + } + | expr ARITH_MUL expr = { + $$ = $1 * $3; +#ifdef OVERFLOW + if (Oflag && $$/$1 != $3 ) + yyerror("overflow in"); +#endif + } | expr ARITH_DIV expr = { if ($3 == 0) @@ -110,16 +140,24 @@ int -arith(char *s) +is_add_overflow(intmax_t a, intmax_t b, intmax_t s) { - long result; + if (a > 0 && b > 0 && s <= 0) + return 1; + if (a < 0 && b < 0 && s >= 0) + return 1; + return 0; +} +intmax_t +arith(char *s) +{ arith_buf = arith_startbuf = s; INTOFF; - result = yyparse(); + yyparse(); arith_lex_reset(); /* reprime lex */ INTON; - return (result); + return (arith_res); } @@ -143,5 +181,5 @@ char *concat; char **ap; - long i; + intmax_t i; if (argc > 1) { @@ -168,5 +206,5 @@ i = arith(p); - out1fmt("%ld\n", i); + out1fmt("%jd\n", i); return (! i); } diff -u -r -U 2 -b ../sh.old/arith_lex.l ./arith_lex.l --- ../sh.old/arith_lex.l Fri Jul 19 08:38:51 2002 +++ ./arith_lex.l Sun Apr 20 02:37:37 2003 @@ -44,8 +44,9 @@ #endif /* not lint */ +#include <stdint.h> #include "y.tab.h" #include "error.h" -extern int yylval; +extern intmax_t yylval; extern char *arith_buf, *arith_startbuf; #undef YY_INPUT @@ -57,5 +58,5 @@ %% [ \t\n] { ; } -[0-9]+ { yylval = atol(yytext); return(ARITH_NUM); } +[0-9]+ { yylval = strtoll(yytext, NULL, 10); return(ARITH_NUM); } "(" { return(ARITH_LPAREN); } ")" { return(ARITH_RPAREN); } diff -u -r -U 2 -b ../sh.old/expand.c ./expand.c --- ../sh.old/expand.c Fri Jan 17 14:37:03 2003 +++ ./expand.c Sun Apr 20 02:37:37 2003 @@ -46,4 +46,5 @@ #include <sys/time.h> #include <sys/stat.h> +#include <stdint.h> #include <errno.h> #include <dirent.h> @@ -367,5 +368,5 @@ { char *p, *start; - int result; + intmax_t result; int begoff; int quotes = flag & (EXP_FULL | EXP_CASE | EXP_REDIR); @@ -383,8 +384,8 @@ * characters have to be processed left to right. */ -#if INT_MAX / 1000000000 >= 10 || INT_MIN / 1000000000 <= -10 -#error "integers with more than 10 digits are not supported" -#endif - CHECKSTRSPACE(12 - 2, expdest); +//#if INT_MAX / 1000000000 >= 10 || INT_MIN / 1000000000 <= -10 +//#error "integers with more than 10 digits are not supported" +//#endif + CHECKSTRSPACE(MAXINT_LEN, expdest); USTPUTC('\0', expdest); start = stackblock(); @@ -408,5 +409,5 @@ rmescapes(p+2); result = arith(p+2); - fmtstr(p, 12, "%d", result); + fmtstr(p, MAXINT_LEN + 2, "%qd", result); while (*p++) ; diff -u -r -U 2 -b ../sh.old/options.h ./options.h --- ../sh.old/options.h Tue Aug 27 05:36:28 2002 +++ ./options.h Sun Apr 20 02:24:46 2003 @@ -67,6 +67,13 @@ #define Tflag optlist[16].val #define Pflag optlist[17].val +#ifdef OVERFLOW +#define Oflag optlist[18].val +#endif +#ifdef OVERFLOW +#define NOPTS 19 +#else #define NOPTS 18 +#endif struct optent { @@ -96,4 +103,7 @@ { "trapsasync", 'T', 0 }, { "physical", 'P', 0 }, +#ifdef OVERFLOW + { "overflow", 'O', 0 }, +#endif }; #else diff -u -r -U 2 -b ../sh.old/sh.1 ./sh.1 --- ../sh.old/sh.1 Tue Feb 25 13:27:12 2003 +++ ./sh.1 Sun Apr 20 02:52:02 2003 @@ -44,5 +44,5 @@ .Sh SYNOPSIS .Nm -.Op Fl /+abCEefIimnPpsTuVvx +.Op Fl /+abCEefIimnOPpsTuVvx .Op Fl /+o Ar longname .Op Fl c Ar string @@ -226,4 +226,6 @@ execute them. This is useful for checking the syntax of shell scripts. +.It Fl O Li interactive +If compiled with the overflow checks, turn them during arithmetic operations on. .It Fl P Li physical Change the default for the >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200304200021.h3K0LcFv052208>