Skip site navigation (1)Skip section navigation (2)
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>