Subject: Re: bug#8794: (a) uncontroversial fixes (2011-06-06 version)

From: Paul Eggert <[email-masked]>
Date: Mon, 06 Jun 2011 01:39:14 -0700

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: [email-masked]
# target_branch: bzr+ssh://[email-masked]/emacs/trunk
# testament_sha1: c8b36cc2ea7b3f5a15360443ec4a0832655a629c
# timestamp: 2011-06-06 01:28:12 -0700
# base_revision_id: [email-masked]
# 
# Begin patch
=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-06-06 04:54:23 +0000
+++ src/ChangeLog	2011-06-06 06:16:12 +0000
@@ -1,5 +1,52 @@
 2011-06-06  Paul Eggert  <[email-masked]>
 
+	Check for buffer and string overflow more precisely.
+	* buffer.h (BUF_BYTES_MAX): New macro.
+	* lisp.h (STRING_BYTES_MAX): New macro.
+	* alloc.c (Fmake_string):
+	* character.c (string_escape_byte8):
+	* coding.c (coding_alloc_by_realloc):
+	* doprnt.c (doprnt):
+	* editfns.c (Fformat):
+	* eval.c (verror):
+	Use STRING_BYTES_MAX, not MOST_POSITIVE_FIXNUM,
+	since they may not be the same number.
+	* editfns.c (Finsert_char):
+	* fileio.c (Finsert_file_contents):
+	Likewise for BUF_BYTES_MAX.
+
+	* image.c: Use ptrdiff_t, not int, for sizes.
+	(slurp_file): Switch from int to ptrdiff_t.
+	All uses changed.
+	(slurp_file): Check that file size fits in both size_t (for
+	malloc) and ptrdiff_t (for sanity and safety).
+
+	* fileio.c (Fverify_visited_file_modtime): Avoid time overflow
+	if b->modtime has its maximal value.
+
+	* dired.c (Ffile_attributes): Don't assume EMACS_INT has >32 bits.
+
+	Don't assume time_t can fit into int.
+	* buffer.h (struct buffer.modtime): Now time_t, not int.
+	* fileio.c (Fvisited_file_modtime): No need for time_t cast now.
+	* undo.c (Fprimitive_undo): Use time_t, not int, for time_t value.
+
+	Minor fixes for signed vs unsigned integers.
+	* character.h (MAYBE_UNIFY_CHAR):
+	* charset.c (maybe_unify_char):
+	* keyboard.c (read_char, reorder_modifiers):
+	XINT -> XFASTINT, since the integer must be nonnegative.
+	* ftfont.c (ftfont_spec_pattern):
+	* keymap.c (access_keymap, silly_event_symbol_error):
+	XUINT -> XFASTINT, since the integer must be nonnegative.
+	(Fsingle_key_description, preferred_sequence_p): XUINT -> XINT,
+	since it makes no difference and we prefer signed.
+	* keyboard.c (record_char): Use XUINT when all the neighbors do.
+	(access_keymap): NATNUMP -> INTEGERP, since the integer must be
+	nonnegative.
+
+2011-06-06  Paul Eggert  <[email-masked]>
+
 	* alloc.c (memory_full) [SYSTEM_MALLOC]: Port to MacOS (Bug#8800).
 	Do not assume that spare memory exists; that assumption is valid
 	only if SYSTEM_MALLOC.

=== modified file 'src/alloc.c'
--- src/alloc.c	2011-06-06 04:54:23 +0000
+++ src/alloc.c	2011-06-06 06:16:12 +0000
@@ -2211,7 +2211,7 @@
       int len = CHAR_STRING (c, str);
       EMACS_INT string_len = XINT (length);
 
-      if (string_len > MOST_POSITIVE_FIXNUM / len)
+      if (string_len > STRING_BYTES_MAX / len)
 	string_overflow ();
       nbytes = len * string_len;
       val = make_uninit_multibyte_string (string_len, nbytes);

=== modified file 'src/buffer.h'
--- src/buffer.h	2011-05-12 07:07:06 +0000
+++ src/buffer.h	2011-06-06 06:16:12 +0000
@@ -306,6 +306,11 @@
   }								\
 while (0)
 
+/* Maximum number of bytes in a buffer.
+   A buffer cannot contain more bytes than a 1-origin fixnum can represent,
+   nor can it be so large that C pointer arithmetic stops working.  */
+#define BUF_BYTES_MAX min (MOST_POSITIVE_FIXNUM - 1, min (SIZE_MAX, PTRDIFF_MAX))
+
 /* Return the address of byte position N in current buffer.  */
 
 #define BYTE_POS_ADDR(n) \
@@ -545,7 +550,7 @@
      -1 means visited file was nonexistent.
      0 means visited file modtime unknown; in no case complain
      about any mismatch on next save attempt.  */
-  int modtime;
+  time_t modtime;
   /* Size of the file when modtime was set.  This is used to detect the
      case where the file grew while we were reading it, so the modtime
      is still the same (since it's rounded up to seconds) but we're actually

=== modified file 'src/character.c'
--- src/character.c	2011-05-21 04:33:23 +0000
+++ src/character.c	2011-06-06 06:16:12 +0000
@@ -838,7 +838,7 @@
   if (multibyte)
     {
       if ((MOST_POSITIVE_FIXNUM - nchars) / 3 < byte8_count
-	  || (MOST_POSITIVE_FIXNUM - nbytes) / 2 < byte8_count)
+	  || (STRING_BYTES_MAX - nbytes) / 2 < byte8_count)
 	string_overflow ();
 
       /* Convert 2-byte sequence of byte8 chars to 4-byte octal.  */
@@ -847,7 +847,7 @@
     }
   else
     {
-      if ((MOST_POSITIVE_FIXNUM - nchars) / 3 < byte8_count)
+      if ((STRING_BYTES_MAX - nchars) / 3 < byte8_count)
 	string_overflow ();
 
       /* Convert 1-byte sequence of byte8 chars to 4-byte octal.  */

=== modified file 'src/character.h'
--- src/character.h	2011-05-21 04:33:23 +0000
+++ src/character.h	2011-06-01 02:49:12 +0000
@@ -544,7 +544,7 @@
 	Lisp_Object val;				\
 	val = CHAR_TABLE_REF (Vchar_unify_table, c);	\
 	if (INTEGERP (val))				\
-	  c = XINT (val);				\
+	  c = XFASTINT (val);				\
 	else if (! NILP (val))				\
 	  c = maybe_unify_char (c, val);		\
       }							\

=== modified file 'src/charset.c'
--- src/charset.c	2011-05-31 06:05:00 +0000
+++ src/charset.c	2011-06-03 18:11:17 +0000
@@ -1637,7 +1637,7 @@
   struct charset *charset;
 
   if (INTEGERP (val))
-    return XINT (val);
+    return XFASTINT (val);
   if (NILP (val))
     return c;
 
@@ -1647,7 +1647,7 @@
     {
       val = CHAR_TABLE_REF (Vchar_unify_table, c);
       if (! NILP (val))
-	c = XINT (val);
+	c = XFASTINT (val);
     }
   else
     {

=== modified file 'src/coding.c'
--- src/coding.c	2011-05-30 01:12:12 +0000
+++ src/coding.c	2011-06-06 06:16:12 +0000
@@ -1071,8 +1071,8 @@
 static void
 coding_alloc_by_realloc (struct coding_system *coding, EMACS_INT bytes)
 {
-  if (coding->dst_bytes >= MOST_POSITIVE_FIXNUM - bytes)
-    error ("Maximum size of buffer or string exceeded");
+  if (STRING_BYTES_MAX - coding->dst_bytes < bytes)
+    string_overflow ();
   coding->destination = (unsigned char *) xrealloc (coding->destination,
 						    coding->dst_bytes + bytes);
   coding->dst_bytes += bytes;

=== modified file 'src/dired.c'
--- src/dired.c	2011-04-14 19:34:42 +0000
+++ src/dired.c	2011-06-06 05:55:38 +0000
@@ -1013,12 +1013,11 @@
 	 The code on the next line avoids a compiler warning on
 	 systems where st_ino is 32 bit wide. (bug#766).  */
       EMACS_INT high_ino = s.st_ino >> 31 >> 1;
-      EMACS_INT low_ino  = s.st_ino & 0xffffffff;
 
       values[10] = Fcons (make_number (high_ino >> 8),
 			  Fcons (make_number (((high_ino & 0xff) << 16)
-					      + (low_ino >> 16)),
-				 make_number (low_ino & 0xffff)));
+					      + (s.st_ino >> 16 & 0xffff)),
+				 make_number (s.st_ino & 0xffff)));
     }
 
   /* Likewise for device.  */

=== modified file 'src/doprnt.c'
--- src/doprnt.c	2011-04-30 20:05:43 +0000
+++ src/doprnt.c	2011-06-06 06:16:12 +0000
@@ -329,7 +329,7 @@
 		minlen = atoi (&fmtcpy[1]);
 	      string = va_arg (ap, char *);
 	      tem = strlen (string);
-	      if (tem > MOST_POSITIVE_FIXNUM)
+	      if (tem > STRING_BYTES_MAX)
 		error ("String for %%s or %%S format is too long");
 	      width = strwidth (string, tem);
 	      goto doit1;
@@ -338,7 +338,7 @@
 	    doit:
 	      /* Coming here means STRING contains ASCII only.  */
 	      tem = strlen (string);
-	      if (tem > MOST_POSITIVE_FIXNUM)
+	      if (tem > STRING_BYTES_MAX)
 		error ("Format width or precision too large");
 	      width = tem;
 	    doit1:

=== modified file 'src/editfns.c'
--- src/editfns.c	2011-06-05 22:46:26 +0000
+++ src/editfns.c	2011-06-06 06:16:12 +0000
@@ -2342,7 +2342,7 @@
     len = CHAR_STRING (XFASTINT (character), str);
   else
     str[0] = XFASTINT (character), len = 1;
-  if (MOST_POSITIVE_FIXNUM / len < XINT (count))
+  if (BUF_BYTES_MAX / len < XINT (count))
     error ("Maximum buffer size would be exceeded");
   n = XINT (count) * len;
   if (n <= 0)
@@ -3589,7 +3589,7 @@
   char initial_buffer[4000];
   char *buf = initial_buffer;
   EMACS_INT bufsize = sizeof initial_buffer;
-  EMACS_INT max_bufsize = min (MOST_POSITIVE_FIXNUM + 1, SIZE_MAX);
+  EMACS_INT max_bufsize = STRING_BYTES_MAX + 1;
   char *p;
   Lisp_Object buf_save_value IF_LINT (= {0});
   register char *format, *end, *format_start;

=== modified file 'src/eval.c'
--- src/eval.c	2011-05-30 05:39:59 +0000
+++ src/eval.c	2011-06-06 06:16:12 +0000
@@ -1994,7 +1994,7 @@
 {
   char buf[4000];
   size_t size = sizeof buf;
-  size_t size_max = min (MOST_POSITIVE_FIXNUM + 1, SIZE_MAX);
+  size_t size_max = STRING_BYTES_MAX + 1;
   size_t mlen = strlen (m);
   char *buffer = buf;
   size_t used;

=== modified file 'src/fileio.c'
--- src/fileio.c	2011-04-29 19:47:29 +0000
+++ src/fileio.c	2011-06-06 06:16:12 +0000
@@ -3248,7 +3248,7 @@
   /* Check whether the size is too large or negative, which can happen on a
      platform that allows file sizes greater than the maximum off_t value.  */
   if (! not_regular
-      && ! (0 <= st.st_size && st.st_size <= MOST_POSITIVE_FIXNUM))
+      && ! (0 <= st.st_size && st.st_size <= BUF_BYTES_MAX))
     error ("Maximum buffer size exceeded");
 
   /* Prevent redisplay optimizations.  */
@@ -4960,7 +4960,7 @@
   if ((st.st_mtime == b->modtime
        /* If both are positive, accept them if they are off by one second.  */
        || (st.st_mtime > 0 && b->modtime > 0
-	   && (st.st_mtime == b->modtime + 1
+	   && (st.st_mtime - 1 == b->modtime
 	       || st.st_mtime == b->modtime - 1)))
       && (st.st_size == b->modtime_size
           || b->modtime_size < 0))
@@ -4990,7 +4990,7 @@
 {
   if (! current_buffer->modtime)
     return make_number (0);
-  return make_time ((time_t) current_buffer->modtime);
+  return make_time (current_buffer->modtime);
 }
 
 DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,

=== modified file 'src/ftfont.c'
--- src/ftfont.c	2011-04-11 03:39:45 +0000
+++ src/ftfont.c	2011-06-01 02:49:12 +0000
@@ -815,7 +815,7 @@
 	    goto err;
 	  for (chars = XCDR (chars); CONSP (chars); chars = XCDR (chars))
 	    if (CHARACTERP (XCAR (chars))
-		&& ! FcCharSetAddChar (charset, XUINT (XCAR (chars))))
+		&& ! FcCharSetAddChar (charset, XFASTINT (XCAR (chars))))
 	      goto err;
 	}
     }

=== modified file 'src/image.c'
--- src/image.c	2011-05-31 06:05:00 +0000
+++ src/image.c	2011-06-06 06:10:06 +0000
@@ -2112,9 +2112,6 @@
 			      File Handling
  ***********************************************************************/
 
-static unsigned char *slurp_file (char *, int *);
-
-
 /* Find image file FILE.  Look in data-directory/images, then
    x-bitmap-file-path.  Value is the encoded full name of the file
    found, or nil if not found.  */
@@ -2151,7 +2148,7 @@
    occurred.  *SIZE is set to the size of the file.  */
 
 static unsigned char *
-slurp_file (char *file, int *size)
+slurp_file (char *file, ptrdiff_t *size)
 {
   FILE *fp = NULL;
   unsigned char *buf = NULL;
@@ -2159,6 +2156,7 @@
 
   if (stat (file, &st) == 0
       && (fp = fopen (file, "rb")) != NULL
+      && 0 <= st.st_size && st.st_size <= min (PTRDIFF_MAX, SIZE_MAX)
       && (buf = (unsigned char *) xmalloc (st.st_size),
 	  fread (buf, 1, st.st_size, fp) == st.st_size))
     {
@@ -2814,7 +2812,7 @@
     {
       Lisp_Object file;
       unsigned char *contents;
-      int size;
+      ptrdiff_t size;
 
       file = x_find_image_file (file_name);
       if (!STRINGP (file))
@@ -4039,7 +4037,7 @@
     {
       Lisp_Object file;
       unsigned char *contents;
-      int size;
+      ptrdiff_t size;
 
       file = x_find_image_file (file_name);
       if (!STRINGP (file))
@@ -5021,6 +5019,7 @@
 
   if (stat (SDATA (file), &st) == 0
       && (fp = fopen (SDATA (file), "rb")) != NULL
+      && 0 <= st.st_size && st.st_size <= min (PTRDIFF_MAX, SIZE_MAX)
       && (buf = (char *) xmalloc (st.st_size),
 	  fread (buf, 1, st.st_size, fp) == st.st_size))
     {
@@ -5055,7 +5054,7 @@
   enum {PBM_MONO, PBM_GRAY, PBM_COLOR} type;
   unsigned char *contents = NULL;
   unsigned char *end, *p;
-  int size;
+  ptrdiff_t size;
 
   specified_file = image_spec_value (img->spec, QCfile, NULL);
 
@@ -7869,7 +7868,7 @@
 static int svg_load (struct frame *f, struct image *img);
 
 static int svg_load_image (struct frame *, struct image *,
-                           unsigned char *, unsigned int);
+                           unsigned char *, ptrdiff_t);
 
 /* The symbol `svg' identifying images of this type. */
 
@@ -8047,7 +8046,7 @@
     {
       Lisp_Object file;
       unsigned char *contents;
-      int size;
+      ptrdiff_t size;
 
       file = x_find_image_file (file_name);
       if (!STRINGP (file))
@@ -8096,7 +8095,7 @@
 svg_load_image (struct frame *f,         /* Pointer to emacs frame structure.  */
 		struct image *img,       /* Pointer to emacs image structure.  */
 		unsigned char *contents, /* String containing the SVG XML data to be parsed.  */
-		unsigned int size)       /* Size of data in bytes.  */
+		ptrdiff_t size)          /* Size of data in bytes.  */
 {
   RsvgHandle *rsvg_handle;
   RsvgDimensionData dimension_data;

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2011-06-04 07:41:44 +0000
+++ src/keyboard.c	2011-06-06 05:48:28 +0000
@@ -2395,8 +2395,8 @@
 
       c = Faref (Vexecuting_kbd_macro, make_number (executing_kbd_macro_index));
       if (STRINGP (Vexecuting_kbd_macro)
-	  && (XINT (c) & 0x80) && (XUINT (c) <= 0xff))
-	XSETFASTINT (c, CHAR_META | (XINT (c) & ~0x80));
+	  && (XFASTINT (c) & 0x80) && (XFASTINT (c) <= 0xff))
+	XSETFASTINT (c, CHAR_META | (XFASTINT (c) & ~0x80));
 
       executing_kbd_macro_index++;
 
@@ -3321,7 +3321,7 @@
       if (INTEGERP (c))
 	{
 	  if (XUINT (c) < 0x100)
-	    putc (XINT (c), dribble);
+	    putc (XUINT (c), dribble);
 	  else
 	    fprintf (dribble, " 0x%"pI"x", XUINT (c));
 	}
@@ -6370,7 +6370,7 @@
   Lisp_Object parsed;
 
   parsed = parse_modifiers (symbol);
-  return apply_modifiers ((int) XINT (XCAR (XCDR (parsed))),
+  return apply_modifiers (XFASTINT (XCAR (XCDR (parsed))),
 			  XCAR (parsed));
 }

=== modified file 'src/keymap.c'
--- src/keymap.c	2011-05-12 07:07:06 +0000
+++ src/keymap.c	2011-06-01 02:49:12 +0000
@@ -462,7 +462,7 @@
     XSETFASTINT (idx, XINT (idx) & (CHAR_META | (CHAR_META - 1)));
 
   /* Handle the special meta -> esc mapping. */
-  if (INTEGERP (idx) && XUINT (idx) & meta_modifier)
+  if (INTEGERP (idx) && XFASTINT (idx) & meta_modifier)
     {
       /* See if there is a meta-map.  If there's none, there is
          no binding for IDX, unless a default binding exists in MAP.  */
@@ -480,7 +480,7 @@
       if (CONSP (event_meta_map))
 	{
 	  map = event_meta_map;
-	  idx = make_number (XUINT (idx) & ~meta_modifier);
+	  idx = make_number (XFASTINT (idx) & ~meta_modifier);
 	}
       else if (t_ok)
 	/* Set IDX to t, so that we only find a default binding.  */
@@ -529,7 +529,7 @@
 	  }
 	else if (VECTORP (binding))
 	  {
-	    if (NATNUMP (idx) && XFASTINT (idx) < ASIZE (binding))
+	    if (INTEGERP (idx) && XFASTINT (idx) < ASIZE (binding))
 	      val = AREF (binding, XFASTINT (idx));
 	  }
 	else if (CHAR_TABLE_P (binding))
@@ -537,7 +537,7 @@
 	    /* Character codes with modifiers
 	       are not included in a char-table.
 	       All character codes without modifiers are included.  */
-	    if (NATNUMP (idx) && (XFASTINT (idx) & CHAR_MODIFIER_MASK) == 0)
+	    if (INTEGERP (idx) && (XFASTINT (idx) & CHAR_MODIFIER_MASK) == 0)
 	      {
 		val = Faref (binding, idx);
 		/* `nil' has a special meaning for char-tables, so
@@ -1357,7 +1357,7 @@
   int modifiers;
 
   parsed = parse_modifiers (c);
-  modifiers = (int) XUINT (XCAR (XCDR (parsed)));
+  modifiers = XFASTINT (XCAR (XCDR (parsed)));
   base = XCAR (parsed);
   name = Fsymbol_name (base);
   /* This alist includes elements such as ("RET" . "\\r").  */
@@ -2416,7 +2416,7 @@
     {
       char tem[KEY_DESCRIPTION_SIZE];
 
-      *push_key_description (XUINT (key), tem, 1) = 0;
+      *push_key_description (XINT (key), tem, 1) = 0;
       return build_string (tem);
     }
   else if (SYMBOLP (key))	/* Function key or event-symbol */
@@ -2515,7 +2515,7 @@
 	return 0;
       else
 	{
-	  int modifiers = XUINT (elt) & (CHAR_MODIFIER_MASK & ~CHAR_META);
+	  int modifiers = XINT (elt) & (CHAR_MODIFIER_MASK & ~CHAR_META);
 	  if (modifiers == where_is_preferred_modifier)
 	    result = 2;
 	  else if (modifiers)

=== modified file 'src/lisp.h'
--- src/lisp.h	2011-06-02 08:25:28 +0000
+++ src/lisp.h	2011-06-06 06:16:12 +0000
@@ -763,6 +763,12 @@
 
 #endif /* not GC_CHECK_STRING_BYTES */
 
+/* A string cannot contain more bytes than a fixnum can represent,
+   nor can it be so long that C pointer arithmetic stops working on
+   the string plus a terminating null.  */
+#define STRING_BYTES_MAX  \
+  min (MOST_POSITIVE_FIXNUM, min (SIZE_MAX, PTRDIFF_MAX) - 1)
+
 /* Mark STR as a unibyte string.  */
 #define STRING_SET_UNIBYTE(STR)  \
   do { if (EQ (STR, empty_multibyte_string))  \

=== modified file 'src/undo.c'
--- src/undo.c	2011-04-14 05:04:02 +0000
+++ src/undo.c	2011-06-02 06:15:15 +0000
@@ -500,7 +500,7 @@
 		{
 		  /* Element (t high . low) records previous modtime.  */
 		  Lisp_Object high, low;
-		  int mod_time;
+		  time_t mod_time;
 		  struct buffer *base_buffer = current_buffer;
 
 		  high = Fcar (cdr);

# Begin bundle