Bash 2.05 builtins mishandle integers >2**31 on 64-bit hosts

[Paul Eggert]

Configuration Information [Automatically generated, do not change]:
Machine: sparc
OS: solaris2.7
Compiler: cc -xarch=v9
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='sparc' 
-DCONF_OSTYPE='solaris2.7' -DCONF_MACHTYPE='sparc-sun-solaris2.7' 
-DCONF_VENDOR='sun' -DSHELL  -DHAVE_CONFIG_H  -D_LARGEFILE_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -I.  -I.. -I../include -I../lib 
-I/tmp/prefix/include -g
uname output: SunOS sic.twinsun.com 5.7 Generic_106541-15 sun4u sparc 
SUNW,UltraSPARC-IIi-Engine
Machine Type: sparc-sun-solaris2.7

Bash Version: 2.05
Patch Level: 0
Release Status: release

Description:
        Several Bash builtins mishandle integers larger than 2**31 on
        64-bit hosts, where Bash arithmetic is normally 64-bit.
        These builtins include "break", "continue", "history", "pushd",
        and "shift".

Repeat-By:
        Here is how to reproduce the problem with "break".
        Put the following text into a file named "test".

                testbreak() {
                  for i in 1 2
                  do
                    for j in 1 2
                    do
                      break $1
                      : no break
                    done
                    : done with inner loop
                  done
                  : done with outer loop
                }
                testbreak 1000
                testbreak 4294967296
                testbreak 4294967297
                testbreak 9223372036854775809
                testbreak 9223372036854775807

        Then run the command "bash -x test".  Here is the output
        that I get, on 64-bit Solaris sparc:

                ./bash -x $j/test
                + testbreak 1000
                + break 1000
                + : done with outer loop
                + testbreak 4294967296
                + break 4294967296
                test: break: loop count must be > 0
                + : done with outer loop
                + testbreak 4294967297
                + break 4294967297
                + : done with inner loop
                + break 4294967297
                + : done with inner loop
                + : done with outer loop
                + testbreak 9223372036854775809
                + break 9223372036854775809
                test: break: loop count must be > 0
                + : done with outer loop
                + testbreak 9223372036854775807
                + break 9223372036854775807
                test: break: loop count must be > 0
                + : done with outer loop

        In the above example, only "break 1000" was handled correctly;
        the other "break" commands were mishandled in one way or another.

        Here is what the output should look like:

                + testbreak 1000
                + break 1000
                + : done with outer loop
                + testbreak 4294967296
                + break 4294967296
                + : done with outer loop
                + testbreak 4294967297
                + break 4294967297
                + : done with outer loop
                + testbreak 9223372036854775809
                + break 9223372036854775809
                + : done with outer loop
                + testbreak 9223372036854775807
                + break 9223372036854775807
                + : done with outer loop

Fix:

2001-04-28  Paul Eggert  <eggert@twinsun.com>

        Fix some long->int conversion bugs in builtins like "break"
        that accept integer arguments.

        * builtins/break.def (break_builtin, continue_builtin): Don't
        discard information about converting the argument to 'int'.
        Leave it 'long'.
        * builtins/shift.def (shift_builtin): Likewise.
        * builtins/history.def (history_builtin): Likewise.
        Convert arg to 'int' only if it is in range.
        (display_history): Likewise.
        
        * builtins/pushd.def (get_dirstack_index, get_dirstack_element,
        set_dirstack_element): ind is now 'long', not 'int'.
        (get_dirstack_element):  Return -1 if 'ind' is not in range.

        * builtins/common.c (get_numeric_arg): Don't discard
        information by returning 'int'.  Return 'long' instead.

        * builtins/common.h (get_numeric_arg): Return long, not int.
        (get_dirstack_element, set_dirstack_element):
        First arg is long, not int.

===================================================================
RCS file: builtins/break.def,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/break.def  2001/02/21 17:03:00     2.5
+++ builtins/break.def  2001/04/29 00:22:00     2.5.0.1
@@ -59,7 +59,7 @@ int
 break_builtin (list)
      WORD_LIST *list;
 {
-  int newbreak;
+  long newbreak;
 
   if (check_loop_level () == 0)
     return (EXECUTION_SUCCESS);
@@ -94,7 +94,7 @@ int
 continue_builtin (list)
      WORD_LIST *list;
 {
-  int newcont;
+  long newcont;
 
   if (check_loop_level () == 0)
     return (EXECUTION_SUCCESS);
===================================================================
RCS file: builtins/common.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/common.c   2001/02/14 22:05:11     2.5
+++ builtins/common.c   2001/04/29 00:22:00     2.5.0.1
@@ -337,7 +337,7 @@ set_dollar_vars_changed ()
    follow.  If FATAL is true, call throw_to_top_level, which exits the
    shell; if not, call jump_to_top_level (DISCARD), which aborts the
    current command. */
-int
+long
 get_numeric_arg (list, fatal)
      WORD_LIST *list;
      int fatal;
===================================================================
RCS file: builtins/common.h,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/common.h   2000/10/13 15:52:55     2.5
+++ builtins/common.h   2001/04/29 00:22:00     2.5.0.1
@@ -49,7 +49,7 @@ extern int dollar_vars_changed __P((void
 extern void set_dollar_vars_unchanged __P((void));
 extern void set_dollar_vars_changed __P((void));
 
-extern int get_numeric_arg __P((WORD_LIST *, int));
+extern long get_numeric_arg __P((WORD_LIST *, int));
 extern int read_octal __P((char *));
 
 /* Keeps track of the current working directory. */
@@ -93,8 +93,8 @@ extern void set_var_attribute __P((char 
 
 /* Functions from pushd.def */
 extern char *get_dirstack_from_string __P((char *));
-extern char *get_dirstack_element __P((int, int));
-extern void set_dirstack_element __P((int, int, char *));
+extern char *get_dirstack_element __P((long, int));
+extern void set_dirstack_element __P((long, int, char *));
 extern WORD_LIST *get_directory_stack __P((void));
 
 /* Functions from evalstring.c */
===================================================================
RCS file: builtins/history.def,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/history.def        2001/02/14 22:09:42     2.5
+++ builtins/history.def        2001/04/29 00:22:00     2.5.0.1
@@ -163,17 +163,14 @@ history_builtin (list)
 #endif
   else if (flags & DFLAG)
     {
-      if (legal_number (delete_arg, &delete_offset) == 0)
+      if (!legal_number (delete_arg, &delete_offset)
+         || delete_offset < history_base
+         || delete_offset > (history_base + history_length))
        {
          builtin_error ("%s: not a valid history position", delete_arg);
          return (EXECUTION_FAILURE);
        }
       opt = delete_offset;
-      if (opt < history_base || opt < 0 || opt > (history_base + 
history_length))
-       {
-         builtin_error ("%d: not a valid history position", opt);
-         return (EXECUTION_FAILURE);
-       }
       result = delete_histent (opt - history_base);
       /* Since remove_history changes history_length, this can happen if
         we delete the last history entry. */
@@ -217,16 +214,17 @@ display_history (list)
      WORD_LIST *list;
 {
   register int i;
-  int limited, limit;
+  long limit;
   HIST_ENTRY **hlist;
 
   if (list)
     {
-      limited = 1;
       limit = get_numeric_arg (list, 0);
+      if (limit < 0)
+       limit = -limit;
     }
   else
-    limited = limit = 0;
+    limit = -1;
 
   hlist = history_list ();
 
@@ -235,10 +233,9 @@ display_history (list)
       for (i = 0;  hlist[i]; i++)
        ;
 
-      if (limit < 0)
-       limit = -limit;
-
-      if ((limited == 0)  || ((i -= limit) < 0))
+      if (0 <= limit && limit < i)
+       i -= limit;
+      else
        i = 0;
 
       while (hlist[i])
===================================================================
RCS file: builtins/pushd.def,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/pushd.def  2001/02/14 22:10:37     2.5
+++ builtins/pushd.def  2001/04/29 00:22:00     2.5.0.1
@@ -537,7 +537,8 @@ add_dirstack_element (dir)
 
 static int
 get_dirstack_index (ind, sign, indexp)
-     int ind, sign, *indexp;
+     long ind;
+     int sign, *indexp;
 {
   if (indexp)
     *indexp = sign > 0 ? 1 : 2;
@@ -552,8 +553,10 @@ get_dirstack_index (ind, sign, indexp)
        *indexp = sign > 0 ? 2 : 1;
       return 0;
     }
-  else
+  else if (0 <= ind && ind <= directory_list_offset)
     return (sign > 0 ? directory_list_offset - ind : ind);
+  else
+    return (-1);
 }
 
 /* Used by the tilde expansion code. */
@@ -586,7 +589,8 @@ get_dirstack_from_string (string)
 #ifdef INCLUDE_UNUSED
 char *
 get_dirstack_element (ind, sign)
-     int ind, sign;
+     long ind;
+     int sign;
 {
   int i;
 
@@ -598,7 +602,8 @@ get_dirstack_element (ind, sign)
 
 void
 set_dirstack_element (ind, sign, value)
-     int ind, sign;
+     long ind;
+     int sign;
      char *value;
 {
   int i;
===================================================================
RCS file: builtins/shift.def,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- builtins/shift.def  1999/08/05 11:44:38     2.5
+++ builtins/shift.def  2001/04/29 00:22:00     2.5.0.1
@@ -52,7 +52,7 @@ int
 shift_builtin (list)
      WORD_LIST *list;
 {
-  int times;
+  long times;
   register int count;
   WORD_LIST *temp;