* alloc.c: Catch some string size overflows that we were missing.

(XMALLOC_OVERRUN_CHECK_SIZE) [!XMALLOC_OVERRUN_CHECK]: Define to 0,
for convenience in STRING_BYTES_MAX.
(STRING_BYTES_MAX): New macro, superseding the old one in lisp.h.
The definition here is exact; the one in lisp.h was approximate.
(allocate_string_data): Check for string overflow.  This catches
some instances we weren't catching before.  Also, it catches
size_t overflow on (unusual) hosts where SIZE_MAX <= min
(PTRDIFF_MAX, MOST_POSITIVE_FIXNUM), e.g., when size_t is 32 bits
and ptrdiff_t and EMACS_INT are both 64 bits.
* character.c, coding.c, doprnt.c, editfns.c, eval.c:
All uses of STRING_BYTES_MAX replaced by STRING_BYTES_BOUND.
* lisp.h (STRING_BYTES_BOUND): Renamed from STRING_BYTES_MAX.

8 files changed, 54 insertions, 13 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index b05b3603c0d..64f346baa98 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,19 @@
+2011-06-08  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* alloc.c: Catch some string size overflows that we were missing.
+	(XMALLOC_OVERRUN_CHECK_SIZE) [!XMALLOC_OVERRUN_CHECK]: Define to 0,
+	for convenience in STRING_BYTES_MAX.
+	(STRING_BYTES_MAX): New macro, superseding the old one in lisp.h.
+	The definition here is exact; the one in lisp.h was approximate.
+	(allocate_string_data): Check for string overflow.  This catches
+	some instances we weren't catching before.  Also, it catches
+	size_t overflow on (unusual) hosts where SIZE_MAX <= min
+	(PTRDIFF_MAX, MOST_POSITIVE_FIXNUM), e.g., when size_t is 32 bits
+	and ptrdiff_t and EMACS_INT are both 64 bits.
+	* character.c, coding.c, doprnt.c, editfns.c, eval.c:
+	All uses of STRING_BYTES_MAX replaced by STRING_BYTES_BOUND.
+	* lisp.h (STRING_BYTES_BOUND): Renamed from STRING_BYTES_MAX.
+
 2011-06-07  Paul Eggert  <eggert@cs.ucla.edu>
 
 	* character.c (string_escape_byte8): Fix nbytes/nchars typo.
diff --git a/src/alloc.c b/src/alloc.c
index db1744bc7cc..fa4f1d38130 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -485,7 +485,9 @@ buffer_memory_full (EMACS_INT nbytes)
 }
 
 
-#ifdef XMALLOC_OVERRUN_CHECK
+#ifndef XMALLOC_OVERRUN_CHECK
+#define XMALLOC_OVERRUN_CHECK_SIZE 0
+#else
 
 /* Check for overrun in malloc'ed buffers by wrapping a 16 byte header
    and a 16 byte trailer around each block.
@@ -1659,6 +1661,18 @@ static char const string_overrun_cookie[GC_STRING_OVERRUN_COOKIE_SIZE] =
 
 #define GC_STRING_EXTRA (GC_STRING_OVERRUN_COOKIE_SIZE)
 
+/* Exact bound on the number of bytes in a string, not counting the
+   terminating null.  A string cannot contain more bytes than
+   STRING_BYTES_BOUND, nor can it be so long that the size_t
+   arithmetic in allocate_string_data would overflow while it is
+   calculating a value to be passed to malloc.  */
+#define STRING_BYTES_MAX					  \
+  min (STRING_BYTES_BOUND,					  \
+       ((SIZE_MAX - XMALLOC_OVERRUN_CHECK_SIZE - GC_STRING_EXTRA  \
+	 - offsetof (struct sblock, first_data)			  \
+	 - SDATA_DATA_OFFSET)					  \
+	& ~(sizeof (EMACS_INT) - 1)))
+
 /* Initialize string allocation.  Called from init_alloc_once.  */
 
 static void
@@ -1858,6 +1872,9 @@ allocate_string_data (struct Lisp_String *s,
   struct sblock *b;
   EMACS_INT needed, old_nbytes;
 
+  if (STRING_BYTES_MAX < nbytes)
+    string_overflow ();
+
   /* Determine the number of bytes needed to store NBYTES bytes
      of string data.  */
   needed = SDATA_SIZE (nbytes);
diff --git a/src/character.c b/src/character.c
index fe8be7084f0..aae3e3c0de6 100644
--- a/src/character.c
+++ b/src/character.c
@@ -838,7 +838,7 @@ string_escape_byte8 (Lisp_Object string)
   if (multibyte)
     {
       if ((MOST_POSITIVE_FIXNUM - nchars) / 3 < byte8_count
-	  || (STRING_BYTES_MAX - nbytes) / 2 < byte8_count)
+	  || (STRING_BYTES_BOUND - nbytes) / 2 < byte8_count)
 	string_overflow ();
 
       /* Convert 2-byte sequence of byte8 chars to 4-byte octal.  */
@@ -847,7 +847,7 @@ string_escape_byte8 (Lisp_Object string)
     }
   else
     {
-      if ((STRING_BYTES_MAX - nbytes) / 3 < byte8_count)
+      if ((STRING_BYTES_BOUND - nbytes) / 3 < byte8_count)
 	string_overflow ();
 
       /* Convert 1-byte sequence of byte8 chars to 4-byte octal.  */
diff --git a/src/coding.c b/src/coding.c
index 64e8e41a5a1..d914e0d641b 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -1071,7 +1071,7 @@ coding_set_destination (struct coding_system *coding)
 static void
 coding_alloc_by_realloc (struct coding_system *coding, EMACS_INT bytes)
 {
-  if (STRING_BYTES_MAX - coding->dst_bytes < bytes)
+  if (STRING_BYTES_BOUND - coding->dst_bytes < bytes)
     string_overflow ();
   coding->destination = (unsigned char *) xrealloc (coding->destination,
 						    coding->dst_bytes + bytes);
diff --git a/src/doprnt.c b/src/doprnt.c
index 5ca3ea89be6..f5e31153628 100644
--- a/src/doprnt.c
+++ b/src/doprnt.c
@@ -329,7 +329,7 @@ doprnt (char *buffer, register size_t bufsize, const char *format,
 		minlen = atoi (&fmtcpy[1]);
 	      string = va_arg (ap, char *);
 	      tem = strlen (string);
-	      if (tem > STRING_BYTES_MAX)
+	      if (STRING_BYTES_BOUND < tem)
 		error ("String for %%s or %%S format is too long");
 	      width = strwidth (string, tem);
 	      goto doit1;
@@ -338,7 +338,7 @@ doprnt (char *buffer, register size_t bufsize, const char *format,
 	    doit:
 	      /* Coming here means STRING contains ASCII only.  */
 	      tem = strlen (string);
-	      if (tem > STRING_BYTES_MAX)
+	      if (STRING_BYTES_BOUND < tem)
 		error ("Format width or precision too large");
 	      width = tem;
 	    doit1:
diff --git a/src/editfns.c b/src/editfns.c
index b4ce9a1c571..232af3595d0 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -3589,7 +3589,7 @@ usage: (format STRING &rest OBJECTS)  */)
   char initial_buffer[4000];
   char *buf = initial_buffer;
   EMACS_INT bufsize = sizeof initial_buffer;
-  EMACS_INT max_bufsize = STRING_BYTES_MAX + 1;
+  EMACS_INT max_bufsize = STRING_BYTES_BOUND + 1;
   char *p;
   Lisp_Object buf_save_value IF_LINT (= {0});
   register char *format, *end, *format_start;
diff --git a/src/eval.c b/src/eval.c
index ef5abac17ae..a9703fc0aa0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1994,7 +1994,7 @@ verror (const char *m, va_list ap)
 {
   char buf[4000];
   size_t size = sizeof buf;
-  size_t size_max = STRING_BYTES_MAX + 1;
+  size_t size_max = STRING_BYTES_BOUND + 1;
   size_t mlen = strlen (m);
   char *buffer = buf;
   size_t used;
diff --git a/src/lisp.h b/src/lisp.h
index c5f810a0746..a1bc794ead5 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -765,11 +765,19 @@ extern EMACS_INT string_bytes (struct Lisp_String *);
 
 #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)
+/* An upper bound on the number of bytes in a Lisp string, not
+   counting the terminating null.  This a tight enough bound to
+   prevent integer overflow errors that would otherwise occur during
+   string size calculations.  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 its terminating null.
+   Although the actual size limit (see STRING_BYTES_MAX in alloc.c)
+   may be a bit smaller than STRING_BYTES_BOUND, calculating it here
+   would expose alloc.c internal details that we'd rather keep
+   private.  The cast to ptrdiff_t ensures that STRING_BYTES_BOUND is
+   signed.  */
+#define STRING_BYTES_BOUND  \
+  min (MOST_POSITIVE_FIXNUM, (ptrdiff_t) min (SIZE_MAX, PTRDIFF_MAX) - 1)
 
 /* Mark STR as a unibyte string.  */
 #define STRING_SET_UNIBYTE(STR)  \